Skip to content

Conversation

@dagar
Copy link
Member

@dagar dagar commented Nov 25, 2020

  • split out switches from manual_control_setpoint into new message manual_control_switches
  • manual_control_switches published at minimal rate (~ 1 Hz) or immediately on change
  • simple switch debounce in rc_update (2 consecutive identical decodes required)
  • commander now polls manual_control_switches rather than sleeping revisit later
  • manual_control_switches logged at full rate rather than sampled at (5-10% of messages logged)
  • processing of mode_slot and mode_switch is now split so we only do one or the other (not both)
    • a future step will be to finally drop mode_switch and accompanying switches entirely

Capture

FYI @RicardoM17 @mhkabir @MaEtUgR

@dagar dagar force-pushed the pr-manual_control_switches_simple branch 2 times, most recently from 47da5b8 to 197a44a Compare November 25, 2020 01:07
@dagar dagar force-pushed the pr-manual_control_switches_simple branch 2 times, most recently from 41bef1e to ca3ea83 Compare November 25, 2020 02:50
@dagar dagar force-pushed the pr-manual_control_switches_simple branch from ca3ea83 to 62a97ef Compare November 26, 2020 14:28
@dagar
Copy link
Member Author

dagar commented Nov 26, 2020

This is ready for review, but won't be mergable until we fix the flash shortage.

@dagar dagar marked this pull request as ready for review November 26, 2020 14:29
@dagar dagar requested a review from MaEtUgR November 26, 2020 14:29
@dagar dagar force-pushed the pr-manual_control_switches_simple branch 5 times, most recently from 9d5925f to eaa37e0 Compare December 8, 2020 19:44
@dagar dagar force-pushed the pr-manual_control_switches_simple branch from eaa37e0 to be275ba Compare December 8, 2020 19:56
@dagar dagar requested a review from cmic0 December 8, 2020 20:02
@dagar dagar added this to the Release v1.12.0 milestone Dec 8, 2020
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Nice improvements, good that you went after the ancient rc_update 👍 It was not easy to go through because of the one commit reordering, indentation, new logic.

I found some issues, the blocking ones to fix being:

  • Old stick value usage in auto land because of missing timeout
  • Any switch change resulting in mode switch evaluation

@dagar
Copy link
Member Author

dagar commented Dec 11, 2020

Bench tested basic mode switches (mode slot), kill switch, etc. https://logs.px4.io/plot_app?log=955b2728-4981-40ef-bc4d-c272c8d8371a

@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 11, 2020

@dagar Thanks for addressing the comments!
Three things open:

@dagar
Copy link
Member Author

dagar commented Dec 11, 2020

Flight Review compatibility (forgot to mention yesterday):

Yes I was prepared to jump on that as soon as this lands in master.

I can make #15949 ready rebased on this pr such that we don't have multiple breaking changes.

Let's go for it.

@dagar dagar force-pushed the pr-manual_control_switches_simple branch from 5e8f16f to 6013fcc Compare December 11, 2020 15:05
@dagar dagar requested a review from MaEtUgR December 11, 2020 15:44
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

It looks correct to me now. Also the latest changes to the landing gear logic.
Missing is the flight review adaption that could theoretically go together with #15949 at least I made the pr ready for review.

@dagar
Copy link
Member Author

dagar commented Dec 11, 2020

Thanks @MaEtUgR for the very thorough code review.

@dagar dagar merged commit ef6209b into master Dec 11, 2020
@dagar dagar deleted the pr-manual_control_switches_simple branch December 11, 2020 17:11
bkueng added a commit to PX4/flight_review that referenced this pull request Jan 14, 2021
bkueng added a commit to PX4/flight_review that referenced this pull request Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants