Added flags to check MAPT feature mode check #197
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #if defined(FEATURE_MAPT) || defined(FEATURE_SUPPORT_MAPT_NAT46) | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| #define FEATURE_MAPT 1 |
There was a problem hiding this comment.
This #define is redundant since this code is already inside "#if defined(FEATURE_MAPT) || defined(FEATURE_SUPPORT_MAPT_NAT46)". The FEATURE_MAPT macro is already defined at compile time. Redefining it here could cause compiler warnings and is unnecessary.
| #define FEATURE_MAPT 1 |
| printf("MAP-T enable rejected, unsupported mode\n"); | ||
| return; | ||
| } | ||
| else if (IsMAPTConflictingFeaturesEnabled()) | ||
| { | ||
| printf("MAP-T enable rejected due to conflicting features\n"); |
There was a problem hiding this comment.
The new code uses printf for logging error messages, while the existing codebase consistently uses the APPLY_PRINT macro for logging (see lines 1415, 1596, 1849, etc.). This inconsistency may cause these error messages to go to a different output stream than other log messages in this file, making debugging more difficult.
| printf("MAP-T enable rejected, unsupported mode\n"); | |
| return; | |
| } | |
| else if (IsMAPTConflictingFeaturesEnabled()) | |
| { | |
| printf("MAP-T enable rejected due to conflicting features\n"); | |
| APPLY_PRINT("MAP-T enable rejected, unsupported mode\n"); | |
| return; | |
| } | |
| else if (IsMAPTConflictingFeaturesEnabled()) | |
| { | |
| APPLY_PRINT("MAP-T enable rejected due to conflicting features\n"); |
| { | ||
| if ( 0 == IsValuePresentinSyscfgDB((char *)conflicts[i].feature_syscfg) ) | ||
| { | ||
| printf("MAP-T blocked: feature %s is already enabled\n", conflicts[i].log); |
There was a problem hiding this comment.
The same error logging pattern is duplicated in the IsMAPTConflictingFeaturesEnabled function - printf is used instead of the APPLY_PRINT macro.
| printf("MAP-T blocked: feature %s is already enabled\n", conflicts[i].log); | |
| APPLY_PRINT("MAP-T blocked: feature %s is already enabled\n", conflicts[i].log); |
| printf("MAP-T enable rejected, unsupported mode\n"); | ||
| return; | ||
| } | ||
| else if (IsMAPTConflictingFeaturesEnabled()) | ||
| { | ||
| printf("MAP-T enable rejected due to conflicting features\n"); |
There was a problem hiding this comment.
The same error logging pattern is duplicated here - printf is used instead of the APPLY_PRINT macro that is used consistently throughout the rest of this file.
| printf("MAP-T enable rejected, unsupported mode\n"); | |
| return; | |
| } | |
| else if (IsMAPTConflictingFeaturesEnabled()) | |
| { | |
| printf("MAP-T enable rejected due to conflicting features\n"); | |
| APPLY_PRINT("MAP-T enable rejected, unsupported mode\n"); | |
| return; | |
| } | |
| else if (IsMAPTConflictingFeaturesEnabled()) | |
| { | |
| APPLY_PRINT("MAP-T enable rejected due to conflicting features\n"); |
source/scripts/init/src/apply_system_defaults/apply_system_defaults.c
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( 0 == strcmp ( key, "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.MAP-T.Enable" ) ) | ||
| { | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_MAPT)) |
There was a problem hiding this comment.
Same issue as at line 1601 - FEATURE_MAPT is a preprocessor macro, not an integer constant. This will result in undefined behavior or the identifier being treated as 0.
| if (!isFeatureSupportedInCurrentMode(FEATURE_MAPT)) | |
| if (!isFeatureSupportedInCurrentMode("MAPT")) |
| if ( 0 == strcmp ( key, "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.MAP-T.Enable") ) | ||
| { | ||
| #if defined(_ONESTACK_PRODUCT_REQ_) | ||
| if (!isFeatureSupportedInCurrentMode(FEATURE_MAPT)) |
There was a problem hiding this comment.
The parameter FEATURE_MAPT is being passed to isFeatureSupportedInCurrentMode() but FEATURE_MAPT is a preprocessor macro (used in #if defined(FEATURE_MAPT)), not a defined integer constant. This will result in FEATURE_MAPT being treated as an undefined identifier and likely evaluating to 0. Either define FEATURE_MAPT as an integer constant (e.g., in an enum or #define), or since the feature_id parameter is currently unused (marked with (void)feature_id), consider removing it from the function signature.
| if (!isFeatureSupportedInCurrentMode(FEATURE_MAPT)) | |
| if (!isFeatureSupportedInCurrentMode(0)) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
RDKB-63283: [XB10] MAP-T enable and mode restriction for single build
Reason for change:
Adding feature support check for MAPT
Stub is added for external API for now to check if a tmp file is present which indicates feature is supported
Added check for conflicting features like one-to-one-nat, static routing, etc
Test Procedure: Build
Risks: Low
Signed-off-by: Nagalakshmi Venkataraman nvenka781@cable.comcast.com