RDKB-63406:Devicetype RFC default value is not migrating after software upgrade#212
RDKB-63406:Devicetype RFC default value is not migrating after software upgrade#212NareshM1702 wants to merge 17 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a one-time migration during boot to ensure the DeviceType syscfg value is set to PROD after a software/firmware upgrade (tracked via a new devicetype_migrate syscfg flag) for XB6 and HUB4 platforms.
Changes:
- Add
devicetype_migrategating logic to run the migration only once after upgrade. - If migration hasn’t run yet, set
DeviceTypetoPRODand persist the migration flag.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| source/scripts/init/system/utopia_init_xb6.sh | Adds one-time DeviceType migration logic gated by devicetype_migrate. |
| source/scripts/init/system/utopia_init_hub4.sh | Adds the same one-time DeviceType migration logic for HUB4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| syscfg commit | ||
| else | ||
| echo "DeviceType is already PROD, no change needed" | ||
| fi | ||
| syscfg set devicetype_migrate "1" | ||
| syscfg commit |
There was a problem hiding this comment.
syscfg commit is called twice in this migration path (once after setting DeviceType and again after setting devicetype_migrate). Consider setting both keys first and committing once to reduce persistent storage writes during boot.
| DEVICETYPE_MIGRATE="$(syscfg get devicetype_migrate)" | ||
| echo "$DEVICETYPE_MIGRATE" | ||
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "$CURRENT_DEVICETYPE" |
There was a problem hiding this comment.
The unconditional echo statements output raw syscfg values to stdout during init (including a blank line when unset). For XB6 this script already appends init/migration logs to $Log_file; consider removing these debug prints or logging with a consistent prefix and redirecting to the appropriate log file.
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| syscfg commit | ||
| else | ||
| echo "DeviceType is already PROD, no change needed" | ||
| fi | ||
| syscfg set devicetype_migrate "1" | ||
| syscfg commit |
There was a problem hiding this comment.
syscfg commit is called twice in this migration path (once after setting DeviceType and again after setting devicetype_migrate). To reduce flash/NVRAM writes, set both keys first and commit once at the end of the block.
| DEVICETYPE_MIGRATE="$(syscfg get devicetype_migrate)" | ||
| echo "$DEVICETYPE_MIGRATE" | ||
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "$CURRENT_DEVICETYPE" |
There was a problem hiding this comment.
The unconditional echo statements print raw syscfg values to stdout during init (including a blank line when unset). Consider removing these debug prints or using the same tagged logging style used elsewhere in this script (e.g., echo "[utopia][init] ...").
8964ae6 to
9591b2c
Compare
9591b2c to
e941e3b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "SSH: Forward SSH changed to disabled" >> $Log_file | ||
| fi | ||
|
|
||
| #Change devicetype on firmware udgrade |
There was a problem hiding this comment.
Typo in comment: "udgrade" should be "upgrade".
| #Change devicetype on firmware udgrade | |
| #Change devicetype on firmware upgrade |
| echo "$DEVICETYPE_MIGRATE" | ||
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "$CURRENT_DEVICETYPE" |
There was a problem hiding this comment.
The two echo lines print raw syscfg values to stdout without any context/prefix, which can clutter boot logs and makes it hard to trace where the output came from. Consider removing these debug prints or logging with a consistent prefix (and/or redirecting to the existing $Log_file used in this section).
| echo "$DEVICETYPE_MIGRATE" | |
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | |
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | |
| echo "$CURRENT_DEVICETYPE" | |
| echo "[utopia][init] devicetype_migrate value: $DEVICETYPE_MIGRATE" >> "$Log_file" | |
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | |
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | |
| echo "[utopia][init] current DeviceType value: $CURRENT_DEVICETYPE" >> "$Log_file" |
| syscfg set DeviceType "PROD" | ||
| syscfg commit | ||
| else | ||
| echo "DeviceType is already PROD, no change needed" | ||
| fi | ||
| syscfg set devicetype_migrate "1" | ||
| syscfg commit |
There was a problem hiding this comment.
devicetype_migrate is set unconditionally after the DeviceType update attempt. If syscfg set/commit for DeviceType fails for any reason, the migrate flag will still be written and the script won’t retry on the next boot. Consider setting both keys and doing a single syscfg commit, and only marking devicetype_migrate=1 after confirming the DeviceType change (and commit) succeeded.
| syscfg set DeviceType "PROD" | |
| syscfg commit | |
| else | |
| echo "DeviceType is already PROD, no change needed" | |
| fi | |
| syscfg set devicetype_migrate "1" | |
| syscfg commit | |
| if syscfg set DeviceType "PROD" && syscfg set devicetype_migrate "1" && syscfg commit; then | |
| echo "DeviceType successfully migrated to PROD and devicetype_migrate marked" | |
| else | |
| echo "Failed to migrate DeviceType to PROD; devicetype_migrate not marked (will retry on next boot)" | |
| fi | |
| else | |
| echo "DeviceType is already PROD, no change needed" | |
| if syscfg set devicetype_migrate "1" && syscfg commit; then | |
| echo "devicetype_migrate successfully marked with existing PROD DeviceType" | |
| else | |
| echo "Failed to mark devicetype_migrate with existing PROD DeviceType (will retry on next boot)" | |
| fi | |
| fi |
| syscfg commit | ||
| fi | ||
|
|
||
| #Change devicetype on firmware udgrade |
There was a problem hiding this comment.
Typo in comment: "udgrade" should be "upgrade".
| #Change devicetype on firmware udgrade | |
| #Change devicetype on firmware upgrade |
| echo "$DEVICETYPE_MIGRATE" | ||
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "$CURRENT_DEVICETYPE" |
There was a problem hiding this comment.
The two echo lines print raw syscfg values to stdout without any context/prefix, which can clutter boot logs and makes it hard to trace where the output came from. Consider removing these debug prints or logging with a consistent prefix (e.g., [utopia][init]).
| echo "$DEVICETYPE_MIGRATE" | |
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | |
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | |
| echo "$CURRENT_DEVICETYPE" | |
| echo "[utopia][init] devicetype_migrate flag: $DEVICETYPE_MIGRATE" | |
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | |
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | |
| echo "[utopia][init] Current DeviceType: $CURRENT_DEVICETYPE" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #Change devicetype on firmware udgrade | ||
| DEVICETYPE_MIGRATE="$(syscfg get devicetype_migrate)" | ||
| echo "$DEVICETYPE_MIGRATE" |
There was a problem hiding this comment.
echo "$DEVICETYPE_MIGRATE" looks like leftover debug output and will print an empty line (or "1") to the console/log on every boot. Consider removing it or replacing it with a prefixed message that only logs when a migration is performed.
| echo "$DEVICETYPE_MIGRATE" |
| echo "[utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" |
There was a problem hiding this comment.
Logging tag/format is inconsistent with the rest of this script (nearby messages use "[utopia][init]"), but this block uses "[utopia]" and unprefixed echos. Please align the log prefix/format so migration logs are searchable and consistent.
| echo "[Utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" |
There was a problem hiding this comment.
Logging tag is inconsistent with the rest of this script (nearby messages use "[utopia][init]"), but this block uses "[Utopia]" and unprefixed echos. Please align the log prefix/format so migration logs are searchable and consistent.
|
|
||
| #Change devicetype on firmware udgrade | ||
| DEVICETYPE_MIGRATE="$(syscfg get devicetype_migrate)" | ||
| echo "$DEVICETYPE_MIGRATE" |
There was a problem hiding this comment.
echo "$DEVICETYPE_MIGRATE" looks like leftover debug output and will print an empty line (or "1") to the console/log on every boot. Consider removing it or replacing it with a prefixed message that only logs when a migration is performed.
| echo "$DEVICETYPE_MIGRATE" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3194779 to
91dc5ca
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "[utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| else |
There was a problem hiding this comment.
This change updates syscfg key "DeviceType", but within this repo the runtime DeviceType that affects behavior is read from TR-181 RFC (e.g., service_sshd.sh reads Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Identity.DeviceType via dmcli). If the reported issue is about the RFC/TR-181 DeviceType default not migrating after upgrade, this syscfg write may not have any effect—please confirm the intended backing store and update the correct parameter as needed.
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "[utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| else |
There was a problem hiding this comment.
This change updates syscfg key "DeviceType", but within this repo the runtime DeviceType that affects behavior is read from TR-181 RFC (e.g., service_sshd.sh reads Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Identity.DeviceType via dmcli). If the reported issue is about the RFC/TR-181 DeviceType default not migrating after upgrade, this syscfg write may not have any effect—please confirm the intended backing store and update the correct parameter as needed.
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "[Utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| else |
There was a problem hiding this comment.
This change updates syscfg key "DeviceType", but within this repo the runtime DeviceType that affects behavior is read from TR-181 RFC (e.g., service_sshd.sh reads Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Identity.DeviceType via dmcli). If the reported issue is about the RFC/TR-181 DeviceType default not migrating after upgrade, this syscfg write may not have any effect—please confirm the intended backing store and update the correct parameter as needed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Change device type on firmware upgrade | ||
| DEVICETYPE_MIGRATE="$(syscfg get devicetype_migrate)" | ||
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "[Utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| else | ||
| echo "DeviceType is already PROD, no change needed" | ||
| fi | ||
| syscfg set devicetype_migrate "1" | ||
| syscfg commit | ||
| fi |
There was a problem hiding this comment.
There is a significant discrepancy between the PR description and the code changes. The PR description states "clearing shellcheck errors in utopia components" as the reason for change, but the actual changes add new DeviceType migration logic to handle RFC default value migration after software upgrade. This is not fixing shellcheck errors but adding new functionality. The PR description should be updated to accurately reflect that this change adds DeviceType migration logic, not shellcheck fixes.
| # Change device type on firmware upgrade | ||
| DEVICETYPE_MIGRATE="$(syscfg get devicetype_migrate)" | ||
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo "[Utopia] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then | ||
| echo "setting DeviceType to PROD" | ||
| syscfg set DeviceType "PROD" | ||
| else | ||
| echo "DeviceType is already PROD, no change needed" | ||
| fi | ||
| syscfg set devicetype_migrate "1" | ||
| syscfg commit | ||
| fi |
There was a problem hiding this comment.
The utopia_init_arm.sh file in the same directory has a similar structure (including lan_domain migration at line 248-254) but does not include the DeviceType migration logic that was added to the other three init scripts. If this omission is intentional because ARM platforms don't require this migration, this should be documented in comments. Otherwise, consider adding the same DeviceType migration logic to utopia_init_arm.sh for consistency across all platform init scripts to ensure all devices migrate to the PROD DeviceType after firmware upgrade.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ -z "$DEVICETYPE_MIGRATE" ]; then | ||
| CURRENT_DEVICETYPE="$(syscfg get DeviceType)" | ||
| echo_t "[utopia][init] Devicetype is $CURRENT_DEVICETYPE" | ||
| if [ "$CURRENT_DEVICETYPE" != "PROD" ]; then |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
61f885b to
7779484
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
)" This reverts commit 5a05e85.
7779484 to
80c3369
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change:migrating devicetype RFC after firmware upgrade
Test Procedure: build and flash the image verify the RFC is changing
Risks:Medium
Priority: P1