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

px4fmu split safety button into new module #10119

Merged
merged 2 commits into from
Aug 24, 2019
Merged

px4fmu split safety button into new module #10119

merged 2 commits into from
Aug 24, 2019

Conversation

dagar
Copy link
Member

@dagar dagar commented Aug 2, 2018

TODO:

  • review PWM_SERVO_SET_FORCE_SAFETY_OFF/PWM_SERVO_SET_FORCE_SAFETY_ON
    - completely replace these IOCTLs with in_esc_calibration_mode
  • test pixracer
  • test nxphlite
  • test pixhawk 4 mini

@dagar dagar requested a review from bkueng August 2, 2018 04:12
@dagar dagar force-pushed the pr-safety_button branch from 2ae204d to ae826f1 Compare August 2, 2018 14:33
@dagar dagar changed the base branch from fmu_safety_fix to master August 2, 2018 14:33
@dagar dagar force-pushed the pr-safety_button branch 2 times, most recently from b3b7fbb to dfbe0cc Compare August 4, 2018 15:35
@dagar dagar force-pushed the pr-safety_button branch from dfbe0cc to 0babf5a Compare August 18, 2018 02:35
@dagar dagar mentioned this pull request Sep 13, 2018
@LorenzMeier
Copy link
Member

@bkueng Could you help to get this in? Thanks!

davids5
davids5 previously approved these changes Sep 28, 2018
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

This good as an extract of the code from the FMU. Consider possibly Fix tabbing

Long term there should be board_read_saftey_switch() state or a macro that returns logic true when down.

As it is and since there is a led blinker the poling it fine but longterm
The switch code could be like TAP power button and use interrupts. Then the HP work can then just blink the lights by schedule the changes.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Generally looks good, but there are several details that should to be improved.

src/drivers/safety_button/CMakeLists.txt Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
src/drivers/safety_button/safety.cpp Outdated Show resolved Hide resolved
@dagar dagar force-pushed the pr-safety_button branch 2 times, most recently from 147f357 to cfd20b9 Compare January 30, 2019 15:12
@dagar dagar added this to the Release v1.9.0 milestone Jan 30, 2019
@dagar dagar force-pushed the pr-safety_button branch 4 times, most recently from 1668315 to a8375c8 Compare February 5, 2019 21:07
@PX4 PX4 deleted a comment from stale bot Jul 15, 2019
@stale stale bot removed the Admin: Wont fix label Jul 15, 2019
@julianoes
Copy link
Contributor

@dagar I'm fighting for all your PRs 😄.

@dagar dagar force-pushed the pr-safety_button branch 2 times, most recently from ef973ce to f2fa6d2 Compare August 23, 2019 15:20
@dagar dagar modified the milestones: Release v1.9.0, Release v1.10.0 Aug 23, 2019
@dagar dagar added the bug label Aug 23, 2019
@dagar dagar requested a review from bkueng August 23, 2019 15:22
@dagar dagar force-pushed the pr-safety_button branch 2 times, most recently from 3d7a98c to bf8b5dc Compare August 23, 2019 15:31
@LorenzMeier
Copy link
Member

What is required to bring this in in terms of review / testing?

@dagar
Copy link
Member Author

dagar commented Aug 24, 2019

Verified fmu-v4 behavior (with and without CBRK_IO_SAFETY set) matches master. I'll check fmu-v5 with and without an IO next.

@dagar
Copy link
Member Author

dagar commented Aug 24, 2019

Tested pixhawk 4 and 4 mini.

This mostly preserves the behavior in master, but gets the logic out of the pwm output module (px4fmu). The difference is that we still get safety button functionality independent of fmu running or mixers loaded.

@davids5 and I briefly discussed handling the button with gpio_setevent, but we can pursue that later.

@dagar dagar merged commit b1d59c8 into master Aug 24, 2019
@dagar dagar deleted the pr-safety_button branch August 24, 2019 21:14
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.

5 participants