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_Common: better handle negative intervals #29243

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robertlong13
Copy link
Collaborator

Treat all negative interval values like -1. This is preferable to the current behavior, where we treat those like 1000Hz requests.

Treat all negative interval values like -1.
@robertlong13
Copy link
Collaborator Author

GCSs shouldn't send request for negative rates, but this came about because this Mission Planner bug I introduced: ArduPilot/MissionPlanner#3487

@timtuxworth
Copy link
Contributor

timtuxworth commented Feb 6, 2025

I'm not so sure the default should be "stop sending" messages. Perhaps invalid values should be ignored?

@@ -3127,7 +3127,7 @@ MAV_RESULT GCS_MAVLINK::set_message_interval(uint32_t msg_id, int32_t interval_u
// at.
interval_ms = 0;
}
} else if (interval_us == -1) {
} else if (interval_us < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
} else if (interval_us < 0) {
} else if (interval_us == -1) {

@@ -3127,7 +3127,7 @@ MAV_RESULT GCS_MAVLINK::set_message_interval(uint32_t msg_id, int32_t interval_u
// at.
interval_ms = 0;
}
} else if (interval_us == -1) {
} else if (interval_us < 0) {
// minus-one is "stop sending"
interval_ms = 0;
} else if (interval_us < 1000) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
} else if (interval_us < 1000) {
} else if (interval_us < 0) {
return MAV_RESULT_DENIED;
} else if (interval_us < 1000) {

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.

3 participants