RDK-61088: Replace vendor reboot reason script from shell to C#43
Open
INajeemaBegam wants to merge 1 commit into
Open
RDK-61088: Replace vendor reboot reason script from shell to C#43INajeemaBegam wants to merge 1 commit into
INajeemaBegam wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the reboot-reason fetcher API by simplifying get_hardware_reason() to no longer take a RebootInfo*, and updates the in-tree call sites/tests accordingly.
Changes:
- Changed
get_hardware_reason()signature to accept only(const EnvContext*, HardwareReason*). - Updated
update-prev-reboot-infomain flow and the gtest call sites to use the new signature. - Adjusted argument validation/logging in
get_hardware_reason()to match the new parameters.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
unittest/reboot_log_parser_gtest.cpp |
Updates unit tests to call the new get_hardware_reason() signature (but leaves some now-misleading assertions). |
reboot-reason-fetcher/src/rebootreason_main.c |
Updates the production call site to the new signature. |
reboot-reason-fetcher/src/log_parser.c |
Updates the get_hardware_reason() definition and its argument validation/logging. |
reboot-reason-fetcher/include/update-reboot-info.h |
Updates the exported header declaration for the new function signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int parse_device_properties(EnvContext *ctx); | ||
| void free_env_context(EnvContext *ctx); | ||
| int get_hardware_reason(const EnvContext *ctx, HardwareReason *hwReason, RebootInfo *info); | ||
| int get_hardware_reason(const EnvContext *ctx, HardwareReason *hwReason); |
|
|
||
| RDK_LOG(RDK_LOG_DEBUG,"LOG.RDK.REBOOTINFO","Getting hardware reboot reason for current boot \n"); | ||
| get_hardware_reason(&ctx, &hwReason, &rebootInfo); | ||
| get_hardware_reason(&ctx, &hwReason); |
Comment on lines
+653
to
656
| EXPECT_EQ(get_hardware_reason(&ctx, &hw), SUCCESS); | ||
| EXPECT_STREQ(hw.mappedReason, "UNKNOWN"); | ||
| EXPECT_EQ(info.reason[0], '\0'); | ||
| } |
Comment on lines
669
to
674
| strncpy(info.timestamp, "2026-03-10T11:22:33Z", sizeof(info.timestamp) - 1); | ||
|
|
||
| EXPECT_EQ(get_hardware_reason(&ctx, &hw, &info), SUCCESS); | ||
| EXPECT_EQ(get_hardware_reason(&ctx, &hw), SUCCESS); | ||
| EXPECT_STREQ(hw.mappedReason, "UNKNOWN"); | ||
| EXPECT_EQ(info.reason[0], '\0'); | ||
| EXPECT_STREQ(info.timestamp, "2026-03-10T11:22:33Z"); |
dad4147 to
85e65f7
Compare
85e65f7 to
664741e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
unittest/reboot_log_parser_gtest.cpp:674
- Same issue as the Amlogic variant: with the
RebootInfo*parameter removed, this test’s assertions aboutinfo.reason/info.timestampare no longer meaningful, and the mocked MTK sysfs file isn’t used byget_hardware_reason. Adjust the test to validate the MTK read/mapping behavior viahw(or drop/rename the test if MTK support was intentionally removed fromget_hardware_reason).
TEST_F(LogParserTest, get_hardware_reason_DispatchMtkUsesReadPath) {
setupMockFile("/sys/mtk_pm/boot_reason", "/tmp/reboot_test/mtk_sysfs", "0xD1\n");
g_mock_fs_enabled = true;
EnvContext ctx;
HardwareReason hw;
RebootInfo info;
memset(&ctx, 0, sizeof(ctx));
memset(&hw, 0, sizeof(hw));
memset(&info, 0, sizeof(info));
set_hal_test_soc(&ctx, HalTestSoc::Mediatek);
strncpy(info.timestamp, "2026-03-10T11:22:33Z", sizeof(info.timestamp) - 1);
EXPECT_EQ(get_hardware_reason(&ctx, &hw), SUCCESS);
EXPECT_STREQ(hw.mappedReason, "UNKNOWN");
EXPECT_EQ(info.reason[0], '\0');
EXPECT_STREQ(info.timestamp, "2026-03-10T11:22:33Z");
| utils.c | ||
|
|
||
| librebootnow_la_LIBADD = -lfwutils -lrdkloggers -lrfcapi -lsecure_wrapper | ||
| librebootnow_la_LIBADD = -lfwutils -lrdkloggers -lrfcapi -lsecure_wrapper -lrebootreason |
| rebootnow_LDADD = librebootnow.la | ||
|
|
||
| AM_LDFLAGS = -lrdkloggers -lfwutils -lrfcapi -lsecure_wrapper | ||
| AM_LDFLAGS = -lrdkloggers -lfwutils -lrfcapi -lsecure_wrapper #-lrebootreason |
Comment on lines
641
to
655
| TEST_F(LogParserTest, get_hardware_reason_DispatchAmlogicUsesReadPath) { | ||
| setupMockFile("/sys/devices/platform/aml_pm/reset_reason", "/tmp/reboot_test/amlogic_sysfs", "12\n"); | ||
| g_mock_fs_enabled = true; | ||
|
|
||
| EnvContext ctx; | ||
| HardwareReason hw; | ||
| RebootInfo info; | ||
| memset(&ctx, 0, sizeof(ctx)); | ||
| memset(&hw, 0, sizeof(hw)); | ||
| memset(&info, 0, sizeof(info)); | ||
| set_hal_test_soc(&ctx, HalTestSoc::Amlogic); | ||
|
|
||
| EXPECT_EQ(get_hardware_reason(&ctx, &hw, &info), SUCCESS); | ||
| EXPECT_EQ(get_hardware_reason(&ctx, &hw), SUCCESS); | ||
| EXPECT_STREQ(hw.mappedReason, "UNKNOWN"); | ||
| EXPECT_EQ(info.reason[0], '\0'); |
edbc4e6 to
ec30e74
Compare
Comment on lines
44
to
+52
| @@ -48,13 +48,17 @@ update_prev_reboot_info_SOURCES = \ | |||
| $(top_srcdir)/reboot-reason-fetcher/src/parodus_log_update.c \ | |||
| $(top_srcdir)/reboot-reason-fetcher/src/bootup_reason_checker.c | |||
|
|
|||
| update_prev_reboot_info_la_LIBADD += -lrebootreason | |||
|
|
|||
Comment on lines
53
to
62
| if USE_CPC | ||
| update_prev_reboot_info_SOURCES += \ | ||
| $(srcdir)/rebootmanager-cpc/src/reboot_reason_classify.c \ | ||
| $(srcdir)/rebootmanager-cpc/src/platform_hal.c \ | ||
| $(srcdir)/rebootmanager-cpc/src/log_parser.c | ||
| update_prev_reboot_info_CPPFLAGS = \ | ||
| -I$(srcdir)/rebootmanager-cpc/include | ||
|
|
||
| update_prev_reboot_info_la_LIBADD = -lrebootreason | ||
| else |
Comment on lines
34
to
43
|
|
||
| librebootnow_la_LIBADD = -lfwutils -lrdkloggers -lrfcapi -lsecure_wrapper | ||
| librebootnow_la_LIBADD = -lfwutils -lrdkloggers -lrfcapi -lsecure_wrapper #-lrebootreason | ||
|
|
||
| # CLI wrapper binary | ||
| bin_PROGRAMS = rebootnow | ||
| rebootnow_SOURCES = reboot_main.c | ||
| rebootnow_LDADD = librebootnow.la | ||
|
|
||
| AM_LDFLAGS = -lrdkloggers -lfwutils -lrfcapi -lsecure_wrapper | ||
| AM_LDFLAGS = -lrdkloggers -lfwutils -lrfcapi -lsecure_wrapper #-lrebootreason | ||
|
|
Comment on lines
118
to
122
| int release_lock(const char *lockDir); | ||
| int parse_device_properties(EnvContext *ctx); | ||
| void free_env_context(EnvContext *ctx); | ||
| int get_hardware_reason(const EnvContext *ctx, HardwareReason *hwReason, RebootInfo *info); | ||
| int get_hardware_reason(const EnvContext *ctx, HardwareReason *hwReason); | ||
| int read_brcm_previous_reboot_reason(HardwareReason *hw); |
Comment on lines
+1
to
+74
| ########################################################################## | ||
| # If not stated otherwise in this file or this component's LICENSE | ||
| # file the following copyright and licenses apply: | ||
| # | ||
| # Copyright 2026 Comcast Cable Communications Management, LLC | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| ########################################################################## | ||
|
|
||
| AUTOMAKE_OPTIONS = foreign subdir-objects | ||
|
|
||
| reboot_CFLAGS += -fPIC -pthread | ||
|
|
||
| AM_CPPFLAGS = -I$(top_srcdir)/reboot-helper/include -I$(top_srcdir)/reboot-reason-fetcher/include | ||
| AM_CFLAGS = -std=c99 -Wall -Wextra -Werror -O2 -I./reboot-helper/include -DRDK_LOGGER_ENABLED -fPIC -pthread | ||
|
|
||
| # Library target | ||
| lib_LTLIBRARIES = librebootnow.la | ||
| librebootnow_la_LDFLAGS = -shared | ||
| librebootnow_la_SOURCES = \ | ||
| system_cleanup.c \ | ||
| cyclic_reboot.c \ | ||
| utils.c | ||
|
|
||
| librebootnow_la_LIBADD = -lfwutils -lrdkloggers -lrfcapi -lsecure_wrapper #-lrebootreason | ||
|
|
||
| # CLI wrapper binary | ||
| bin_PROGRAMS = rebootnow | ||
| rebootnow_SOURCES = reboot_main.c | ||
| rebootnow_LDADD = librebootnow.la | ||
|
|
||
| AM_LDFLAGS = -lrdkloggers -lfwutils -lrfcapi -lsecure_wrapper #-lrebootreason | ||
|
|
||
| bin_PROGRAMS += update-prev-reboot-info | ||
| update_prev_reboot_info_SOURCES = \ | ||
| $(top_srcdir)/reboot-reason-fetcher/src/rebootreason_main.c \ | ||
| $(top_srcdir)/reboot-reason-fetcher/src/json.c \ | ||
| $(top_srcdir)/reboot-reason-fetcher/src/parodus_log_update.c \ | ||
| $(top_srcdir)/reboot-reason-fetcher/src/bootup_reason_checker.c | ||
|
|
||
| update_prev_reboot_info_la_LIBADD += -lrebootreason | ||
|
|
||
| if USE_CPC | ||
| update_prev_reboot_info_SOURCES += \ | ||
| $(srcdir)/rebootmanager-cpc/src/reboot_reason_classify.c \ | ||
| $(srcdir)/rebootmanager-cpc/src/platform_hal.c \ | ||
| $(srcdir)/rebootmanager-cpc/src/log_parser.c | ||
| update_prev_reboot_info_CPPFLAGS = \ | ||
| -I$(srcdir)/rebootmanager-cpc/include | ||
|
|
||
| update_prev_reboot_info_la_LIBADD = -lrebootreason | ||
| else | ||
| update_prev_reboot_info_SOURCES += \ | ||
| $(top_srcdir)/reboot-reason-fetcher/src/reboot_reason_classify.c \ | ||
| $(top_srcdir)/reboot-reason-fetcher/src/log_parser.c | ||
| update_prev_reboot_info_CPPFLAGS = \ | ||
| $(AM_CPPFLAGS) \ | ||
| -I$(top_srcdir)/reboot-reason-fetcher/include | ||
| endif | ||
|
|
||
| if IS_TELEMETRY2_ENABLED | ||
| AM_CFLAGS += $(T2_EVENT_FLAG) | ||
| AM_LDFLAGS += -ltelemetry_msgsender -lt2utils | ||
| endif |
Reason for change: Added code for Reboot reason Test Procedure: None Risks: P1 Signed-off-by: najeemabegami <najeemabegam.iqbal@sky.uk>
ec30e74 to
c4006be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Added code for Reboot reason
Test Procedure: None
Risks: P1
Signed-off-by: najeemabegami najeemabegam.iqbal@sky.uk