AP_DroneCAN : add Node Status Logging #23583
Conversation
This will create a log if Option 6 Enable in CAN_OPTION. The Log created will have the information of every Node Health ,Uptime ,and delta between 2 status packet information.
|
@tridge can you review this code. |
|
|
||
| // Log NodeInfo | ||
| void log_NodeStatus(uint8_t node_id, uint32_t uptime_sec, uint8_t healthy, uint8_t mode, uint32_t delta); | ||
| uint32_t last_can_init_delta_ms[126]; |
There was a problem hiding this comment.
this is using a lot of memory for users with the option enabled. I think we should dynamically allocated it, so declare as a pointer here
variable name is also a bit confusing, last_node_status_ms[] would be better
| @@ -416,6 +416,16 @@ void AP_DroneCAN_DNA_Server::handleNodeStatus(const CanardRxTransfer& transfer, | |||
| server_state = HEALTHY; | |||
| } | |||
| } | |||
|
|
|||
| //to calculate the delta b/w handlenode status pkt | |||
| uint32_t now = AP_HAL::native_millis(); | |||
There was a problem hiding this comment.
should be just millis() (its a normal timestamp, not a bus time)
| uint32_t now = AP_HAL::native_millis(); | ||
|
|
||
| if (_ap_dronecan.option_is_set(AP_DroneCAN::Options::LOG_NODE_STATUS)) { | ||
| log_NodeStatus(transfer.source_node_id, msg.uptime_sec, msg.health, msg.mode , now - last_can_init_delta_ms[transfer.source_node_id]); |
There was a problem hiding this comment.
add dynamic allocation if needed
| log_NodeStatus(transfer.source_node_id, msg.uptime_sec, msg.health, msg.mode , now - last_can_init_delta_ms[transfer.source_node_id]); | ||
| } | ||
|
|
||
| last_can_init_delta_ms[transfer.source_node_id] = now; |
There was a problem hiding this comment.
note that node IDs can be up to 127, so allocated array needs to be 128 long. Check this when you put the timestamp in the array, and move this line into if() for option
| */ | ||
| void AP_DroneCAN_DNA_Server::log_NodeStatus(uint8_t node_id, uint32_t uptime_sec, uint8_t healthy, uint8_t mode, uint32_t delta) | ||
| { | ||
| if (node_id > MAX_NODE_ID) { |
There was a problem hiding this comment.
I think we should log up to and including 127, to get the GUI tool when used
|
@robertlong13 I can't tag the original author on this - is this something you're interested in? |
|
@peterbarker this is a PR from my Old carbonix account. This will be a very handy feature to have in ardupilot. |
|
@peterbarker Need your opinion on this PR also comment on the below suggestion. A better idea is the delta should not just be based on NodeStatus but all the packets from the node. Basically, when it is last heard from the node would give us alot of info. |
This will create a log if Option 6 Enable in CAN_OPTION.
The Log created will have the information of every Node Health ,Uptime ,and delta between 2 status packet information.
It is a very good way for diagnosis if the AP_Periph has been failing to send data and also if it has rebooted.