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

Verbose RC Override #371

Open
wants to merge 81 commits into
base: main
Choose a base branch
from
Open

Verbose RC Override #371

wants to merge 81 commits into from

Conversation

BillThePlatypus
Copy link
Contributor

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.

superjax
superjax previously approved these changes Feb 11, 2020
Copy link
Contributor

@superjax superjax left a 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;
Copy link
Contributor

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?

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))
Copy link
Contributor

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

rc_values[i] = 1500;
}
for (int i=4;i<8;i++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

# 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
@bsutherland333
Copy link
Contributor

This also seems like a lot of great work, we should visit revisit this at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants