Add missing getCurrentControlProfile implementation#11528
Add missing getCurrentControlProfile implementation#11528y-decimal wants to merge 1 commit intoiNavFlight:maintenance-9.xfrom
Conversation
Review Summary by QodoImplement missing getCurrentControlProfile function
WalkthroughsDescription• Implements missing getCurrentControlProfile function • Iterates through control profiles to find current match • Returns profile index or 0 if not found Diagramflowchart LR
A["getCurrentControlProfile called"] --> B["Loop through profiles"]
B --> C["Match found?"]
C -->|Yes| D["Return profile index"]
C -->|No| E["Return 0"]
File Changes1. src/main/fc/control_profile.c
|
Code Review by Qodo
1. Ambiguous 0 profile index
|
| uint8_t getCurrentControlProfile(void) { | ||
| for (uint8_t index = 0; index < MAX_CONTROL_PROFILE_COUNT; index++) { | ||
| if (currentControlProfile == controlProfiles(index)) { | ||
| return index; | ||
| } | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
1. Ambiguous 0 profile index 📘 Rule violation ≡ Correctness
getCurrentControlProfile() returns 0 when no matching profile is found, which is indistinguishable from a valid profile index 0. This can cause callers to silently act on the wrong control profile instead of detecting an invalid/no-match state.
Agent Prompt
## Issue description
`getCurrentControlProfile()` returns `0` when no profile matches `currentControlProfile`, which is ambiguous because `0` is also a valid profile index.
## Issue Context
This code runs in embedded/task paths where ambiguous defaults can lead to silent misbehavior.
## Fix Focus Areas
- src/main/fc/control_profile.c[105-113]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Shouldn't it return the control profile, rather than the index? Surely if the index is returned the function should be named getCurrentControlProfileID()? |
|
I didn't add the declaration in the header so I can only guess at the original author's intent. But the header file declares a uint8 as the return type, so I assume it was meant to return the index. I suppose I maybe should've raised this as an issue first instead of jumping straight to a PR so it could be discussed in advance how to deal with it. I'll definitely keep that in mind for the future. Let me know how you want to deal with this o7 |
control_profile.h defines a getCurrentControlProfile, but it was never implemented in control_profile.c, leading to issues for anyone wanting to use this function. This PR adds a simple implementation of said function.
The function iterares over index = 0, index < MAX_CONTROL_PROFILE_COUNT and checks if currentControlProfile == controlProfiles(index) for each index, until it finds a match.
If not match is found, it returns 0 instead.
I'm not sure if returning 0 is the best choice here, since it makes it impossible to tell if the index is genuinely just 0 or if something went wrong. I didn't want to return an arbitrary sentinel value like -1 without first checking in with you though since I'm not sure if there's an established convention or anything like that.