Conversation
Forwards messages of interest from Bristlemouth MAVLink integration to Spotter Makes sure both parts of the message exist before packaging it to be sent to Spotter, will timeout if only part of the 2 messages are sent.
| bm_semaphore_take(ctx.mut, BM_MAX_DELAY_UINT32); | ||
| bm_serial_send_usv_metrics(ctx.metrics); | ||
| ctx.metrics = (bm_serial_usv_metrics_t){0}; | ||
| bm_semaphore_give(ctx.mut); |
There was a problem hiding this comment.
What's the semaphore guarding? Right now it's not used anywhere other than here, so a new message could come in and be in the middle of a write to ctx.metrics while you're sending. Do you want to add it to the write side in package_and_send_usv_metrics?
There was a problem hiding this comment.
This function may be called from the RTOS timer thread (from metrics_timer_cb), or in the middleware task (package_and_send_usv_metrics) this is protecting the ctx.metrics variable.
Yes! This mutex also has to protect ctx.metrics when they are being assigned in package_and_send_usv_metrics, which it is not right now. I will add that in.
victorsowa12
left a comment
There was a problem hiding this comment.
Looks good to me! I think this is a good approach! If we miss either part we will still send the data so decisions can be made upstream based on partial data.
What changed?
Forwards messages of interest from Bristlemouth MAVLink integration to the network co-processor (in this case Sofar Ocean's Spotter).
Makes sure both parts of the message exist before packaging it to be sent to Spotter, will timeout if only part of the 2 messages are sent.
How does it make Bristlemouth better?
Bridges USV integration into Bristlemouth! 😉
Where should reviewers focus?
Because the Ardupilot flight controller publishes all Mavlink messages at a default rate of 1Hz, the two messages of interest would arrive at the same time. But these publishing rates can vary depending on how the flight controller is configured for each of these messages, which has me concerned. So my question is:
Should the
bm_serialmessage be broken up into two separate messages, one for each of the MAVLink messagesIt would be nice to keep this as a single message to prevent adding more messages to
bm_serialMaybe the struct for the
bm_serialmessage can look like:And the timeout can be used to determine when to send the message, i.e. if the second part does not arrive.
Would love feedback on thoughts around this!
(edit: I have went through and implemented the above)
Checklist