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

GCS_MAVLink: Check if set_message_interval is too fast #28144

Merged
merged 3 commits into from
Oct 1, 2024

Conversation

stephendade
Copy link
Contributor

From #28101.

Ardupilot caps the maximum message rate at 80% of SCHED_LOOP_RATE.

If a user requests a rate faster than this via MAV_CMD_SET_MESSAGE_INTERVAL, the command now fails and advises the user to increase SCHED_LOOP_RATE.

This will prevent user confusion as per the linked issue.

Tested in SITL.

@stephendade stephendade force-pushed the loop-interval branch 2 times, most recently from 46bb1c9 to 4e433af Compare September 20, 2024 03:53
@stephendade
Copy link
Contributor Author

Appears to be some issue with COMMAND_INT in the rover autotests. COMMAND_LONG works fine though.

Works OK with MAVProxy:

long SET_MESSAGE_INTERVAL 105 10000
command_int GLOBAL SET_MESSAGE_INTERVAL 0 0 105 10000 0 0 0 0 0

are equivalent.

Maybe some issue with the test ... will investigate further.

@stephendade stephendade force-pushed the loop-interval branch 4 times, most recently from 23c1909 to 686cfba Compare September 22, 2024 21:50
@stephendade
Copy link
Contributor Author

Appears to be an unrelated test failure in DeadreckoningNoAirSpeed in Plane. but otherwise the tests are now happy.

Tools/autotest/arducopter.py Outdated Show resolved Hide resolved
Tools/autotest/vehicle_test_suite.py Outdated Show resolved Hide resolved
libraries/GCS_MAVLink/GCS.h Outdated Show resolved Hide resolved
libraries/GCS_MAVLink/GCS.h Outdated Show resolved Hide resolved
libraries/GCS_MAVLink/GCS.h Outdated Show resolved Hide resolved
@stephendade
Copy link
Contributor Author

stephendade commented Sep 23, 2024

For the autotests, there appears to be an issue with context_set_message_rate_hz where it incorrectly sets the message interval:

AT-0002.6: Sending COMMAND_LONG to (1,1) (MAV_CMD_SET_MESSAGE_INTERVAL=511) (p1=74.000000 p2=10.100010 p3=0.000000 p4=0.000000 p5=0.000000 p6=0.000000  p7=0.000000)
AT-0002.6: ACK received: COMMAND_ACK {command : 511, result : 2, progress : 0, result_param2 : 0, target_system : 250, target_component : 250} (0.000000s)

So it tries to set an interval of 10.100010us (which is way above the SCHED_LOOP_RATE). Best guess there's a missing Hz->usec conversion somewhere ... can't seem to find it though :(

EDIT: This is why some of the copter and rover tests are failing. It's wherever context_set_message_rate_hz is used

libraries/GCS_MAVLink/GCS.h Outdated Show resolved Hide resolved
@stephendade stephendade force-pushed the loop-interval branch 5 times, most recently from 9f0d225 to 5e0e08c Compare September 25, 2024 23:51
@stephendade
Copy link
Contributor Author

@peterbarker, I've fixed up the autotest issues. Appears to be 2x unrelated tests failing

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

LGTM

@stephendade
Copy link
Contributor Author

For the Dev Call, since I likely won't be able to attend:
This was in response to #28101, where a user tried to get a message at a faster rate than 80% of SCHED_LOOP_RATE. This is more apparent in Rover, where SCHED_LOOP_RATE defaults to 50Hz.

libraries/GCS_MAVLink/GCS_Common.cpp Outdated Show resolved Hide resolved
@stephendade stephendade force-pushed the loop-interval branch 2 times, most recently from c6aa679 to 497c27c Compare October 1, 2024 12:04
@peterbarker peterbarker merged commit dad98d9 into ArduPilot:master Oct 1, 2024
95 checks passed
@peterbarker
Copy link
Contributor

Merged, thanks!

@stephendade stephendade deleted the loop-interval branch October 21, 2024 06:10
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.

7 participants