-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fixed swapped ifdefs where disabling Jeti XBUS disables Spektrum SRXL telemetry and vice versa #6442
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
Fixed swapped ifdefs where disabling Jeti XBUS disables Spektrum SRXL telemetry and vice versa #6442
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find @sebsx!
@@ -28,6 +28,10 @@ | |||
#define USBD_PRODUCT_STRING "CrazyBee F3 FR" | |||
#endif | |||
|
|||
// clear up some flash space | |||
#undef USE_PWM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- the cleanup should be done in a separate pull request;
- the list used for cleanup should be the same in all targets, or else managing flash space on F3 will be even more painful as it already is;
- Removed more features from F3 to make the firmware fit into flash. #6444 already takes care of it (but happy to look into using the reductions suggested by you instead, if they are sufficient).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. I reverted the last commit and will submit another PR with the same set of (ideally rarely used) features removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hold on, repurposing of #6444 already in progress.
@mikeller I added another commit to fix overruns on some F3 targets by disabling specific options like PWM input, MAVLINK telemetry and protocols like SUMH and XBUS. Not sure if it's the best approach as we should probably try to keep the same feature set for all the boards in the same family I suppose. Regarding #6444, removing SRXL telemetry is probably not a good idea IMO since modern Spektrum receivers will be using it and moving back to Spektrum2048 means re-wiring the receiver to an RX pin instead of a TX pin (and losing telemetry in the process). |
Wow. Interesting find. How did this slip by inspections? |
src/main/target/common_fc_post.h
Outdated
#undef USE_TELEMETRY_JETIEXBUS | ||
#endif | ||
|
||
#if !defined(USE_SERIALRX_JETIEXBUS) | ||
#if !defined(USE_SERIALRX_SPEKTRUM) | ||
#undef USE_TELEMETRY_SRXL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This #undef
should be moved to the section below with other undefs depending on #ifndef USE_SERIALRX_SPEKTRUM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea
Updated as per latest suggestions. Let me know if a squash is in order. |
@sebsx: Yes please, a rebase is needed to get the flash space reductions in. |
9506916
to
29446a6
Compare
… versa Consolidated disabled features when USE_SERIALRX_SPEKTRUM is disabled
29446a6
to
448a57c
Compare
Should be alright now. |
@mikeller apparently we still have some boards overflowing |
@sebsx: Yes we do. Good thing playing feature whack-a-mole is one of my favourite pasttimes. (It isn't.) Will look into it in a while. |
@mikeller I noticed, but, in case you need a hand I don't mind experimenting a bit. I think we should be able to disable on a board by board basis, for example CRAZYBEEF3FR has an integrated FrSky receiver so there's a slim chance someone hooks it up with a Spektrum or FlySky receiver. IN this case disabling a bunch of protocols should not be a problem. Same with other boards, disabling PWM where there are no PWM input pins, etc (I realise this should be done by the board's maintainer but I guess most older boards are abandoned by now). What do you think? |
@sebsx: I have taken these on board, and integrated them into the list of candidates to be disabled. SUMH is really superseded by SUMD, and I consider PWM to be obsolete as well. |
@mikeller Cheers! 8000 |
…defs Fixed swapped ifdefs where disabling Jeti XBUS disables Spektrum SRXL telemetry and vice versa
Probably a copy/paste error.