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

AP_DroneCAN: properly convert timeout to deadline for aux frames #28308

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

tpwrules
Copy link
Contributor

@tpwrules tpwrules commented Oct 4, 2024

The timeout specified for auxiliary driver frames was passed to the driver where a deadline was expected. The transmission was then started after its "deadline", thereby causing it to be canceled and the data lost if the frame could not be sent immediately.

Fix by converting the timeout to a deadline before passing to the driver. The conversion is done in the Canard interface code as it already does other conversions from timeouts to deadlines.

Tested that my setup used to get timeouts registered in @SYS/can0_stats.txt and doesn't anymore.

The timeout specified for auxiliary driver frames was passed to the
driver where a deadline was expected. The transmission was then started
after its "deadline", thereby causing it to be canceled and the data
lost if the frame could not be sent immediately.

Fix by converting the timeout to a deadline before passing to the
driver. The conversion is done in the Canard interface code as it
already does other conversions from timeouts to deadlines.
Copy link
Contributor

@magicrub magicrub left a comment

Choose a reason for hiding this comment

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

This looks like a correct change to me but lets make sure @bugobliterator or @tridge takes a peek

@tridge tridge merged commit e9e7eba into ArduPilot:master Oct 7, 2024
98 checks passed
@tpwrules tpwrules deleted the pr/can-aux-timeout branch October 7, 2024 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants