PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.38k stars 13.46k forks source link

Re-Implement Auto ESC Index Enumeration Over DroneCAN #21913

Open vertiq-jordan opened 1 year ago

vertiq-jordan commented 1 year ago

Describe problem solved by the proposed feature

Hello all! I am looking to get Auto ESC Index Enumeration back into the PX4 stack. This is a feature that was in the firmware up until Jan 10, 2022 when this PR removed it with only the description, "remove legacy ESC enumeration in FW server." Now, in all of my testing, it would appear that even the video example does not work. QGroundControl sends the correct message, but the flight controller doesn't know what to do with the command: Screenshot 2023-07-31 084818

Running firmware pre-the removal PR, you would get the correct response, and the uavcan.protocol.enumeration.Begin message would be transmitted over the CAN line: Screenshot 2023-07-31 084936

Describe your preferred solution

Ideally, the DroneCAN servers would just revert to their previous versions that included auto ESC enumeration. I believe the last release to have it was 1.12.X. Given Pavel's video example, this implementation works as expected, and is an excellent convenience feature that should still be supported. The PR that took out the feature completely deleted ~800 lines of code, please just re-include those involved with auto enumeration.

Describe possible alternatives

Simply allow the QGroundControl Start Assignment and Stop Assignment buttons to send out the uavcan.protocol.enumeration.Begin command. Allow ESC manufacturers to do as they wish with the received commands. This wouldn't need a server side, just the ability to send the commands.

Additional context

No response

dagar commented 1 year ago

That's funny, I waited years for someone to actually use this in production and it never really materialized.

Are you in a position to test this with a real setup? The commit that removed it won't cleanly revert in place, but is potentially not that hard to restore.

henrykotze commented 1 year ago

I should be in a position to test. All our ESCs are over CAN. This would make our setup so much easier.

vertiq-jordan commented 1 year ago

I am not currently in a position to test on a real setup, unfortunately, sorry. At the moment, the best I could do is verify that the enumeration commands are being transmitted.

dagar commented 1 year ago

For context the motivation for the change is that we wanted the "firmware server" and parameters to always be active and available rather than a special option only available preflight (originally a memory optimization). ESC enumeration happened to be part of this optional piece and since I couldn't find anyone actually using it since the original demo ~ 6 years earlier I decided it wasn't worth keeping.

I'm happy to be wrong if people are actually going to use it now.

vertiq-jordan commented 1 year ago

I find it to be an especially useful feature, one that I'm a little surprised that more ESCs don't support. I can't speak for all people, but I would like to see it added back in.

vertiq-jordan commented 1 year ago

Just poking this for visibility/checking for any updates about decisions being made about reimplementation. Thank you!

Donecle commented 1 year ago

Hello, We were also using it here and would be happy to have it back ! And I'm curious, how do you know if people are actually using features or not? Thanks!