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

[WIP] split drivers/rc_input per protocol #20829

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dagar
Copy link
Member

@dagar dagar commented Dec 23, 2022

WORK IN PROGRESS opening for discussion, testing, and resolving edge cases

Currently in PX4 main for boards that do RC decoding FMU side (ie no px4io) we have a single monolithic RC driver (drivers/rc_input) that includes all protocols and scans input latching once it has a successful decode. Historically this has been mildly convenient for users because typically things have "just worked", but I am now of the opinion that we'd be much better off with explicit configuration and keeping things separate.

Advantages

  • no false positives scanning (eg GHST and CRSF are the same baudrate, checksum, and packet size, but still not compatible)
  • more flexibility to have any RC receiver on any serial port (there are still special cases like PPM or DSM bind)
  • we'll actually know what the system is supposed to have because it was explicitly configured (eg SBUS receiver missing, arming denied)
  • modularity
    • lower risk adding new RC protocols (eg SRXL2 support stalled because it heavily modified the shared driver, a standalone srxl2 driver would have been much lower risk)
    • telemetry and bidirectional support is becoming more common, this gives us more space to expand properly
    • uncommon (arguably legacy) protocols can exist harmlessly off to the side, being entirely removed from builds as needed
  • likely makes it easier to support multiple RC inputs simultaneously (possibly not a real use case, although people have asked)

Disadvantages

  • more code, more flash
  • no more automatic FMU side RC selection

TODO

  • make RC_SERIAL_PORT a board configurable serial port everywhere
  • review and enforce special cases
    • PPM support limited to boards with appropriate hardware
    • DSM binding only on boards and ports with support
    • pre stm32f7/stm32h7 SBUS needs to be limited to boards with hardware support
    • for things like CRSF telemetry do we need to be careful to not allow you to enable telemetry on ports that are RX only or is it harmless
    • oddities on px4_fmu-v5 to test/verify

src/drivers/rc/sumd/module.yaml Outdated Show resolved Hide resolved
src/drivers/rc/st24/module.yaml Outdated Show resolved Hide resolved
src/drivers/rc/sbus/module.yaml Outdated Show resolved Hide resolved
src/drivers/rc/ghst/module.yaml Outdated Show resolved Hide resolved
src/drivers/rc/dsm/module.yaml Outdated Show resolved Hide resolved
@dagar dagar force-pushed the pr-drivers_rc_input_split branch 3 times, most recently from 600a762 to 35ebddb Compare December 23, 2022 22:16
@@ -411,8 +410,10 @@ else
#
. ${R}etc/init.d/rc.serial

# Must be started after the serial config is read
rc_input start $RC_INPUT_ARGS
if param compare -s RC_PPM_EN 1
Copy link
Member

@AlexKlimaj AlexKlimaj Jan 9, 2023

Choose a reason for hiding this comment

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

Looks like RC_PPM_EN isn't actually defined anywhere. It is defined as PPM_IN_EN in the rc_input driver.

@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-community-q-a-may-03-2023/31965/1

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.

None yet

3 participants