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 1 commit 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.


uint8 CONNECTED_ESC_MAX = 8 # The number of ESCs supported. To be consistent with ESC Status, limit it to 8.

DronecanEscStatusExtendedData[8] extended_esc_status_data # An array of up to CONNECTED_ESC_MAX DronecanEscStatusExtendedData instances
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this msg

int16 motor_temperature_deg_c # Temperature of connected motor, in Celsius. Range is -256 to +255 C.
uint16 motor_angle # Measured angle of connected angle sensor, in degrees. Range is 0 to 360.
uint32 status_flags # Manufacturer-specific status flags currently active.
uint8 esc_index # Index of currently reporting ESC.
Copy link
Contributor

Choose a reason for hiding this comment

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

rename DronecanEscStatusExtended.msg


uORB::PublicationMulti<esc_status_s> _esc_status_pub{ORB_ID(esc_status)};
uORB::Publication<dronecan_esc_status_extended_s> _status_extended_pub{ORB_ID(dronecan_esc_status_extended)};
Copy link
Contributor

Choose a reason for hiding this comment

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

change to PublicationMulti

@@ -59,6 +59,7 @@ void LoggedTopics::add_default_topics()
add_topic("config_overrides");
add_topic("cpuload");
add_topic("distance_sensor_mode_change_request");
add_optional_topic("dronecan_esc_status_extended", 250);
Copy link
Contributor

Choose a reason for hiding this comment

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

multi

typedef uavcan::MethodBinder<UavcanEscController *,
void (UavcanEscController::*)(const uavcan::TimerEvent &)> TimerCbBinder;

esc_status_s _esc_status{};
dronecan_esc_status_extended_s _status_extended{};
Copy link
Contributor

Choose a reason for hiding this comment

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

delete

@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!

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