Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorder boxmodes for flight mode priority #10047

Closed
wants to merge 2 commits into from

Conversation

breadoven
Copy link
Collaborator

@breadoven breadoven commented May 16, 2024

Reorders boxmodes to prioritise flight modes within the first 32 entries. This ensures flight modes and more significant modes are included in blackbox flight logs.

Numbering has been removed from the boxId_e enum since it doesn't appear to serve any useful purpose.

Doesn't appear to cause any issues from testing ...

Change affects Blackbox Explorer which will need updating to ensure correct boxmode mapping is used depending on INAV version (Ref Blackbox change iNavFlight/blackbox-log-viewer#101).

@stronnag
Copy link
Collaborator

Why not add an additional field to blackbox?

And will you also provide a PR for INAV blackbox-tools as required?

@breadoven
Copy link
Collaborator Author

Why not add an additional field to blackbox?

Do you mean a separate field for actual "flight" modes and another field for all other modes ?

And will you also provide a PR for INAV blackbox-tools as required?

I have something that works. Not sure if it's the best way of doing it ... but it works.

@stronnag
Copy link
Collaborator

Why not add an additional field to blackbox?

Do you mean a separate field for actual "flight" modes and another field for all other modes ?

I was thinking of maybe an flightModeFlags2 which would be the remaining modes right shifted 32 to fit into another uint32_t; but maybe that's overkill (and a pain on the consumer side).

And will you also provide a PR for INAV blackbox-tools as required?

I have something that works. Not sure if it's the best way of doing it ... but it works.

There is an example in src/parser.c from then the VBAT stuff changed in the middle of the development cycle (so version and data dependent). Or just ping me when this is merged and I'll do it.

@breadoven
Copy link
Collaborator Author

Why not add an additional field to blackbox?

Do you mean a separate field for actual "flight" modes and another field for all other modes ?

I was thinking of maybe an flightModeFlags2 which would be the remaining modes right shifted 32 to fit into another uint32_t; but maybe that's overkill (and a pain on the consumer side).

I was thinking maybe it's overkill to include all the modes given most of the remainder aren't that useful for debugging etc.

And will you also provide a PR for INAV blackbox-tools as required?

I have something that works. Not sure if it's the best way of doing it ... but it works.

There is an example in src/parser.c from then the VBAT stuff changed in the middle of the development cycle (so version and data dependent). Or just ping me when this is merged and I'll do it.

I'll do a PR for the change I made, take it from there.

@mmosca
Copy link
Collaborator

mmosca commented May 17, 2024

Will this impact 7->8 upgrades? If so, we should mention it in the release notes.

@breadoven
Copy link
Collaborator Author

Will this impact 7->8 upgrades? If so, we should mention it in the release notes.

Shouldn't have any impact on users other than certain modes selections such as Cruise being recorded in log files.

@breadoven
Copy link
Collaborator Author

Replaced by #10093.

@breadoven breadoven closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants