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

Integrate Additional Logging of Information Received Over DroneCAN #23882

Conversation

vertiq-jordan
Copy link
Contributor

This PR brings in support for the logging of additional DroneCAN information. First, it allows for the logging of DroneCAN nodes' Node Status information. At the moment, PX4 accepts and monitors node statuses, but it is helpful to be able to review additional information after a flight. Next, I've introduced support for the (fairly new) DroneCAN ESC Status Extended message. Like DroneCAN ESC Status, the Status Extended message provides information about connected DroneCAN ESCs. The extended message adds input and output throttle percentages, motor temperature in degrees Celsius, the ESC index reporting, and additional vendor specific status flags. This PR adds support for both receiving and logging information received by ESCs that report the Status Extended message.

Here are a couple of logs that include logged Node Status and Status Extended from 3 connected ESCs.
The first is an application of logging these two data sets as part of the rest of the basic logging topics: extended_and_node_status_demo.zip

The second has configured the SD card to only log the DroneCAN ESC Status Extended and DroneCAN Node Status messages
extended_and_node_status_logged_only.zip

…tatusExtended message

Initial working logging of the extended ESC status message of one motor

update DroneCAN extended status message to have multiple entries

add messages and start process of logging dronecan node status information

working node status logging

cleanup

last bits of cleanup. Get rid of magic numbers and variable names

update logic for adding to node status

run formatting on uavcan esc

final updates. Update naming to be consistent with esc status extended. update node status logging with appropriate names

update comments

fix name of dronecan_esc_status_extended in logged topics
@dakejahl
Copy link
Contributor

dakejahl commented Nov 4, 2024

Hey Jordan thanks for this. I've taken a closer look at this and I think I see a much cleaner way to do this:

  1. There already exists an Entry struct in NodeStatusMonitor with a NodeStatus memeber. Let's just complete the implementation by adding uptime_sec and vendor_specific_status_code to the NodeStatus struct.

  2. In uavcan_main where you added your logic you can instead call _node_status_monitor.forEachNode to then iterate over, advertise, and publish the status info for each online node. For example:
    https://github.com/PX4/PX4-Autopilot/blob/main/src/drivers/uavcan/uavcan_main.cpp#L1132-L1136

  3. This will require using uORB::PublicationMulti so that each node status gets its own instance of DronecanNodeStatusData and you can delete the DronecanNodeStatus message.

With your current implementation there's the issue that if uavcan_main does not loop quickly enough it will only get the node status from the most recently received message.

@dakejahl
Copy link
Contributor

dakejahl commented Nov 4, 2024

I broke out the NodeStatus portion of this into a separate PR #23890. Can you cherry pick the ESC stuff from here into a new PR?

@vertiq-jordan
Copy link
Contributor Author

I broke out the NodeStatus portion of this into a separate PR #23890. Can you cherry pick the ESC stuff from here into a new PR?

Will do!

@vertiq-jordan
Copy link
Contributor Author

ESC Status Extended portion of this PR is out now @dakejahl, so I

@vertiq-jordan vertiq-jordan reopened this Nov 5, 2024
@vertiq-jordan
Copy link
Contributor Author

ESC Status Extended portion of this PR is out now @dakejahl, so I'm going to kill this one off since we have both parts handled now!

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