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

Add Support for DroneCAN Status Extended Message #23896

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vertiq-jordan
Copy link
Contributor

@vertiq-jordan vertiq-jordan commented Nov 5, 2024

This is the Status Extended portion of a prior PR discussed with @dakejahl here #23882. He has handled the Node Status portion in his own PR here: #23890.

Attached is a log where you can see the ESC Status Extended logging.
log_1_2024-11-5-10-08-06.zip

Here's a quick snapshot of the input/output throttle percentages from 3 attached ESCs. One motor is configured purposefully so that the input and output throttles should not match.
image

@dakejahl
Copy link
Contributor

dakejahl commented Nov 5, 2024

You need to use uORB::PublicationMulti so that each received ESC index has its own publisher. With your current implementation there is only 1 publisher which means all ESC indices are updated on every received CAN message. If an ESC goes offline the data from the last received message will continue to be published, which is not what you want.

Instead remove the msg that is just an array and create a publication multi handle member var. Don't use a member var for the status data.

msg/DronecanEscStatusExtended.msg Outdated Show resolved Hide resolved
msg/DronecanEscStatusExtendedData.msg Outdated Show resolved Hide resolved
src/drivers/uavcan/actuators/esc.hpp Outdated Show resolved Hide resolved
src/modules/logger/logged_topics.cpp Outdated Show resolved Hide resolved
src/drivers/uavcan/actuators/esc.hpp Outdated Show resolved Hide resolved
@dakejahl
Copy link
Contributor

dakejahl commented Nov 5, 2024

also maybe add to the high_rate set in logged_topics.cpp

@vertiq-jordan
Copy link
Contributor Author

You need to use uORB::PublicationMulti so that each received ESC index has its own publisher. With your current implementation there is only 1 publisher which means all ESC indices are updated on every received CAN message. If an ESC goes offline the data from the last received message will continue to be published, which is not what you want.

Instead remove the msg that is just an array and create a publication multi handle member var. Don't use a member var for the status data.

Ah, got it, sorry! I had understood the PublicationMulti as, "There are multiple modules in PX4 who are going to be publishing to this topic," and not necessarily multiple devices that need their own publishers. I think I was just confusing myself by thinking that you might want DSHOT telemetry as well as DroneCAN telemetry at the same time, so you'd have 2 ESC Status reporters. Regardless, I get it now, and will make those changes!

dakejahl
dakejahl previously approved these changes Nov 8, 2024
src/drivers/uavcan/actuators/esc.hpp Show resolved Hide resolved
Copy link
Contributor

@dakejahl dakejahl left a comment

Choose a reason for hiding this comment

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

nitpicky but I would make this less verbose, just makes it slightly easier to read

src/drivers/uavcan/actuators/esc.cpp Outdated Show resolved Hide resolved
src/drivers/uavcan/actuators/esc.cpp Outdated Show resolved Hide resolved
src/drivers/uavcan/actuators/esc.cpp Outdated Show resolved Hide resolved
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.

2 participants