-
Notifications
You must be signed in to change notification settings - Fork 47
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
Verbose RC Override #371
base: main
Are you sure you want to change the base?
Verbose RC Override #371
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.
Overall -- I'm really happy with this. In general, I like how the functions got shorter, and some of the real hairy logic got abstracted.
Thanks, also for writing the unit tests. I hope that it was a little bit easier this time, with the tests split out 😝
I can't remember if we were going to wait for the v1.0 release before merging this.
RCOverrideReason stick_override_reason; | ||
RCOverrideReason offboard_inactive_override_reason; | ||
uint16_t override_mask; | ||
} channel_override_t; |
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.
do we want to migrate to using C++ structs, instead of C structs?
src/command_manager.cpp
Outdated
return override_this_channel; | ||
if(!(muxes[MUX_F].onboard->active)) // The throttle has unique override behavior | ||
rc_override |= OVERRIDE_OFFBOARD_T_INACTIVE; | ||
if(RF_.params_.get_param_int(PARAM_RC_OVERRIDE_TAKE_MIN_THROTTLE)) |
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.
I know that the logic is relatively obvious, but I got lost. A comment would be nice.
also, let's put braces on this if
statement
test/command_manager_test.cpp
Outdated
rc_values[i] = 1500; | ||
} | ||
for (int i=4;i<8;i++) |
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.
formatting
84dd5b9
to
ad240fe
Compare
# Conflicts: # .astylerc # boards/airbourne/airbourne_board.cpp # boards/breezy/breezy_board.cpp # comms/mavlink/mavlink.cpp # comms/mavlink/mavlink.h # include/board.h # include/comm_manager.h # include/command_manager.h # include/controller.h # include/estimator.h # include/interface/comm_link.h # include/mixer.h # include/param.h # include/rc.h # include/rosflight.h # include/util.h # src/comm_manager.cpp # src/command_manager.cpp # src/controller.cpp # src/mixer.cpp # src/param.cpp # src/sensors.cpp # test/command_manager_test.cpp # test/estimator_test.cpp # test/test_board.cpp # test/test_board.h # test/turbotrig_test.cpp
# Conflicts: # boards/airbourne/airbourne_board.h # boards/breezy/breezy_board.h # boards/breezy/flash.h # include/board.h # include/command_manager.h # include/controller.h # include/estimator.h # include/interface/comm_link.h # include/mixer.h # include/param.h # include/rc.h # include/util.h # src/comm_manager.cpp # src/command_manager.cpp # src/controller.cpp # src/mixer.cpp # src/sensors.cpp # test/command_manager_test.cpp # test/common.h # test/estimator_test.cpp # test/turbotrig_test.cpp
This also seems like a lot of great work, we should visit revisit this at some point. |
This changes the RC Override variable in the status message from a boolean to a bitfield, that indicates specifically why RC is overriding offboard controls. This also allows you to tell which channels are overridden. There are corresponding changes to rosflight_io that show this in the status messages.