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_RCTelemetry: add missing CRSF scheduler table entry #28805

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

andyp1per
Copy link
Collaborator

@andyp1per andyp1per commented Dec 4, 2024

Closes #28797

The basic problem was that the VARIO table entry was missing so that when we disabled or changed it we changed a different entry. It also meant the PING entry was never run which is vital to determining RX liveness.

@andyp1per andyp1per added the BUG label Dec 4, 2024
@andyp1per andyp1per changed the title AP_RCTelemetry: add missing scheduler table entry and add checks for correct number of entries AP_RCTelemetry: add missing CRSF scheduler table entry and add checks for correct number of entries Dec 4, 2024
@tpwrules
Copy link
Contributor

tpwrules commented Dec 4, 2024

Can we check _time_slots directly (maybe through like an assert_task_count(count) in the superclass)? That would also catch if two tasks got assigned the same ID or if we overflowed and ran out of task slots.

On the other hand it might optimize less; the compiler might be able to elide the panic right now.

@rmackay9 rmackay9 mentioned this pull request Dec 4, 2024
31 tasks
@andyp1per andyp1per changed the title AP_RCTelemetry: add missing CRSF scheduler table entry and add checks for correct number of entries AP_RCTelemetry: add missing CRSF scheduler table entry Dec 5, 2024
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

Copy link
Contributor

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

LGTM, like the smaller change.

But let's not forget to come back and make this less error-prone.

@tpwrules tpwrules merged commit ad539ff into ArduPilot:master Dec 5, 2024
99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 4.6.0-beta2
Development

Successfully merging this pull request may close these issues.

Serious problem: Crossfire receiver with Plane 4.6.0 beta1: no reconnection after failsafe
4 participants