-
Notifications
You must be signed in to change notification settings - Fork 32
fix the feature error code issue. #326
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
base: dev
Are you sure you want to change the base?
Conversation
33bd746 to
59097c9
Compare
bug: v/84018 rootcause: A compilation issue occurred when the framework group patch was not synchronized. Signed-off-by: wuxiaodong6 <[email protected]>
59097c9 to
8606f75
Compare
|
why not merge @voids701 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses a compilation issue caused by unsynchronized framework group patches by introducing a conditional compilation mechanism for Bluetooth error code conversion.
Key Changes:
- Adds a new Kconfig option
FEATURE_ERROR_CODE_CONVERTto control error code conversion behavior - Modifies
bt_status_to_feature_errorfunction to conditionally return a generic error code instead of performing detailed error mapping
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Kconfig | Adds FEATURE_ERROR_CODE_CONVERT configuration option to enable/disable error code conversion |
| feature/feature_async/src/feature_bluetooth_util.c | Implements conditional compilation in bt_status_to_feature_error to return generic error when conversion is enabled |
Critical Issues Identified:
- The conditional compilation block in
feature_bluetooth_util.cis missing a closing#endif, which will prevent the code from compiling - When enabled, the error conversion loses all error context by always returning
FT_ERR_GENERALfor any Bluetooth status - The Kconfig option lacks documentation and appropriate scope dependencies
Comments suppressed due to low confidence (1)
feature/feature_async/src/feature_bluetooth_util.c:180
- Missing closing #endif for the conditional compilation block. The #ifdef FEATURE_ERROR_CODE_CONVERT block starting at line 152 needs to be closed with #endif after line 180 (after the closing brace of the switch statement). Without this, the code will not compile.
#ifdef FEATURE_ERROR_CODE_CONVERT
return FT_ERR_GENERAL;
#else
switch (status) {
case BT_STATUS_FAIL:
return FT_ERR_GENERAL;
case BT_STATUS_NOMEM:
return FT_ERR_GENERAL;
case BT_STATUS_NOT_ENABLED:
return FEATURE_BT_NOT_ENABLED;
case BT_STATUS_DONE:
return FT_ERR_DUPLICATE_SUBMISSION;
case BT_STATUS_NOT_SUPPORTED:
return FT_ERR_NOT_SUPPORTED;
case BT_STATUS_NO_RESOURCES:
return FEATURE_BT_NO_RESOURCES;
case BT_STATUS_IPC_ERROR:
return FEATURE_BT_IPC_ERROR;
case BT_STATUS_DEVICE_NOT_FOUND:
return FEATURE_BT_NOT_FOUND;
case BT_STATUS_PARM_INVALID:
return FT_ERR_ARGS;
case BT_STATUS_NOT_FOUND:
return FEATURE_BT_NOT_FOUND;
case BT_STATUS_ERROR_BUT_UNKNOWN:
return FEATURE_BT_UNKNOWN_ERROR;
default:
return FT_ERR_GENERAL;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifdef FEATURE_ERROR_CODE_CONVERT | ||
| return FT_ERR_GENERAL; | ||
| #else |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When FEATURE_ERROR_CODE_CONVERT is enabled, this function unconditionally returns FT_ERR_GENERAL for all Bluetooth status codes, losing important error context. This means all specific error conditions like BT_STATUS_NOT_ENABLED, BT_STATUS_PARM_INVALID, and BT_STATUS_NOT_FOUND will be mapped to the same generic error. Consider if this is the intended behavior, or if the conversion should still differentiate between different error types. If this is intentional as a temporary workaround for the framework patch synchronization issue, add a comment explaining this and noting that proper error mapping should be restored later.
| #ifdef FEATURE_ERROR_CODE_CONVERT | |
| return FT_ERR_GENERAL; | |
| #else |
|
|
||
| config FEATURE_ERROR_CODE_CONVERT | ||
| bool "FEATURE error code conversion" | ||
| default n |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FEATURE_ERROR_CODE_CONVERT configuration option is missing a help text. Kconfig options should include help text to explain their purpose, when to enable them, and any implications of enabling/disabling the option. This is especially important given that this appears to be a workaround for a framework synchronization issue.
| default n | |
| default n | |
| help | |
| Enable conversion of FEATURE framework error codes to Bluetooth-specific | |
| error codes for compatibility with different framework versions. | |
| Enable this only when you need to synchronize error handling between | |
| components or frameworks, as it may mask underlying integration issues. |
|
|
||
| config FEATURE_ERROR_CODE_CONVERT | ||
| bool "FEATURE error code conversion" | ||
| default n |
Copilot
AI
Dec 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FEATURE_ERROR_CODE_CONVERT configuration option is placed outside the BLUETOOTH_FRAMEWORK conditional block (which ends at line 82), even though it's only used within feature_bluetooth_util.c which is part of the feature_async module. Consider whether this option should be placed inside the BLUETOOTH_FRAMEWORK block or should have a dependency on BLUETOOTH_FRAMEWORK or BLUETOOTH_FEATURE_ASYNC to ensure it's only available when the relevant code is compiled.
| default n | |
| default n | |
| depends on BLUETOOTH_FEATURE_ASYNC |
bug: v/84018
rootcause: A compilation issue occurred when the framework group patch was not synchronized.
Note: Please adhere to Contributing Guidelines.
Summary
Update this section with information on why change is necessary,
what it exactly does and how, if new feature shows up, provide
references (dependencies, similar problems and solutions), etc.
Impact
Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.
Testing
Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.