-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
FMU: Implement baseline pairing command on safety button press #12794
Conversation
I'll rebase this PR after #10119 goes in. |
@@ -70,8 +83,11 @@ SafetyButton::CheckButton() | |||
_button_counter++; | |||
|
|||
} else if (_button_counter == CYCLE_COUNT) { | |||
// switch to armed state | |||
_safety_btn_off = true; | |||
if (!PX4_MFT_HW_SUPPORTED(PX4_MFT_PX4IO) && !_safety_disabled) { |
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 comment above is out of date, please remove or update it.
_safety_btn_off = true; | ||
if (!PX4_MFT_HW_SUPPORTED(PX4_MFT_PX4IO) && !_safety_disabled) { | ||
// switch to armed state | ||
_safety_btn_off = true; |
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 comment doesn't match what is done, right?
// change to disarmed state and notify | ||
_safety_btn_off = false; | ||
if (!PX4_MFT_HW_SUPPORTED(PX4_MFT_PX4IO) && !_safety_disabled) { | ||
// change to disarmed state and notify |
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.
Again, the comment doesn't seem right.
@@ -136,33 +158,45 @@ SafetyButton::Run() | |||
} | |||
|
|||
// read safety switch input and control safety switch LED at 10Hz | |||
CheckButton(); | |||
bool safety_pressed = CheckButton(); |
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.
Comment above seems outdated.
|
||
uint8_t _button_counter{0}; | ||
uint8_t _blink_counter{0}; | ||
bool _safety_disabled{false}; |
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 naming of these variables is getting confusing. What is _safety_disabled
versus _safetey_off
? This needs to be clear from the names and if not possible from a comment.
@@ -71,15 +73,17 @@ class SafetyButton : public ModuleBase<SafetyButton>, public px4::ScheduledWorkI | |||
|
|||
private: | |||
|
|||
void CheckButton(); | |||
bool CheckButton(); |
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 would rename this method to isButtonPressed()
or hasButtonBeenPressed()
to communicate what a true return value means.
vehicle_command_s vcmd{}; | ||
vcmd.command = vehicle_command_s::VEHICLE_CMD_START_RX_PAIR; | ||
vcmd.timestamp = hrt_absolute_time(); | ||
vcmd.param1 = 10.f; // GCS pairing request handled by a companion. TODO: requires mavlink spec |
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.
@MatejFranceskin fyi, you need to check for this and we need to add it to the mavlink spec.
I brought this into a working state, tested on pixracer and pixhawk 4. |
@bkueng Should we close this or resurrect it? |
eaa2da8
to
17580f4
Compare
Rebased. Auterion has been using this downstream already since some time. |
This initial implementation requires debouncing and a triple-press implementation. This commit should be squashed with those changes into a single feature commit.