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

Fix box ids being truncated in status messages #10707

Closed
wants to merge 1 commit into from

Conversation

mmosca
Copy link
Collaborator

@mmosca mmosca commented Feb 17, 2025

Old code was checking only activeBoxIdCount, instead of total box count.

Array of modes used to setup the bits is using fixed indexes in rc_modes.h.

Any item with index >= activeBoxIdCount would be ignored.

@mmosca
Copy link
Collaborator Author

mmosca commented Feb 17, 2025

@stronnag does this make sense?

@mmosca
Copy link
Collaborator Author

mmosca commented Feb 17, 2025

For context,

CHECK_ACTIVE_BOX second parameter is the index that is used in the array and later in the bits and comes from rc_modes.h enum.

#define CHECK_ACTIVE_BOX(condition, index) do { if (IS_ENABLED(condition)) { activeBoxes[index] = 1; } } while(0)

@stronnag
Copy link
Collaborator

@stronnag does this make sense?

I don't think it makes any difference There are only so many modes set according to Features, the the redirection on the loop means we get them.

@mmosca
Copy link
Collaborator Author

mmosca commented Feb 17, 2025

I think it is late and I need sleep :)
there is a second array there, and that does match the active boxes

@stronnag
Copy link
Collaborator

I think it is late and I need sleep :) there is a second array there, and that does match the active boxes

Actually it may break stuff. Where I previously received MSP2_INAV_STATUS boxflags of 0x0000000020100000 I now receive MSP2_INAV_STATUS boxflags of 0xffffe0020100000, which has bits set beyond the size of reported BOXIDS.

And may break things (or at least stuff that checks beyond the size of BOXIDs (e.g. all 64bits fromMSP2_INAV_STATUS on the assumption unused bits are zero)).

So i don't think this change is necessary (or needed).

@stronnag
Copy link
Collaborator

  • BOXIDS lists the modes available for a partiular FC / settings / Features
  • MSP2_INAV_SETTINGS boxflags gemerates a bit mask pointing into the BOXIDS array (i.e. the set of actrives modes in the BOXIDS list).
  • We should not set bits in MSP2_INAV_SETTINGS boxflags beyond the available modes defined by BOXIDS.
  • The extant code is correct. This patch is NOT necessary.

@mmosca mmosca closed this Feb 18, 2025
@mmosca mmosca deleted the mmosca-pack-box-modes branch February 18, 2025 12:48
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.

2 participants