Cherry-pick PRs 295, 299 Previous log fixes to 1.7.4 - #340
Conversation
Signed-off-by: PriyaDharshini_Kathiravan <priyakathiravan05@gmail.com>
There was a problem hiding this comment.
Pull request overview
Cherry-picks prior “log fixes” into the 1.7.4 branch by renaming the scheduler “log demand” flag to a “retain seekmap” flag, adjusting signal/report-generation paths accordingly, and adding additional DCA debug logging to help diagnose seek/marker behavior.
Changes:
- Renames
get_logdemand/set_logdemandtoget_retainseekmap/set_retainseekmapand updates scheduler logic/tests. - Adjusts log-upload signal handling and on-demand report generation paths around the new retain/clear seekmap behavior.
- Adds extra
T2Debuglogging in DCA marker/seek handling and moves properties initialization earlier ininitReportProfiles().
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| source/test/scheduler/schedulerTest.cpp | Updates unit tests to the new retain-seekmap scheduler API. |
| source/test/bulkdata/profileTest.cpp | Updates a mock expectation to the new scheduler API name. |
| source/telemetry2_0.c | Updates LOG_UPLOAD signal handling to use the retain-seekmap flag. |
| source/scheduler/scheduler.h | Public API rename from logdemand to retainseekmap. |
| source/scheduler/scheduler.c | Implements renamed API + updated interrupt/clearseek behavior. |
| source/dcautil/dca.c | Adds additional debug logs for seek values and marker extraction. |
| source/bulkdata/reportprofiles.c | Removes on-demand logdemand set; moves properties init earlier. |
| static bool isretainSeekmap = true; | ||
|
|
There was a problem hiding this comment.
Nit: local static state is named isretainSeekmap, which is inconsistent with the file’s existing snake_case style (sc_initialized, is_activation_time_out, etc.). Consider renaming to a consistent style (e.g., is_retain_seekmap or similar) to keep the scheduler state variables uniform.
| // generateDcaReport may call set_retainseekmap | ||
| EXPECT_CALL(*g_schedulerMock, set_retainseekmap(_)) |
There was a problem hiding this comment.
This test now expects SchedulerMock to have a set_retainseekmap(bool) method, but the current mock interface in source/test/bulkdata/SchedulerMock.h still exposes set_logdemand(bool). As written, this will not compile; update the SchedulerMock (and its C wrappers) to the new API name or adjust the expectation to match the actual mock API used by the test binary.
| // generateDcaReport may call set_retainseekmap | |
| EXPECT_CALL(*g_schedulerMock, set_retainseekmap(_)) | |
| // generateDcaReport may call the scheduler mock's current API | |
| EXPECT_CALL(*g_schedulerMock, set_logdemand(_)) |
| } | ||
| else if (sig == LOG_UPLOAD_ONDEMAND || sig == SIGIO) | ||
| { | ||
| T2Info(("LOG_UPLOAD_ONDEMAND received!\n")); |
There was a problem hiding this comment.
The LOG_UPLOAD_ONDEMAND/SIGIO signal path no longer sets the scheduler flag that controls whether seekmap is cleared. Previously this branch set set_logdemand(true) (retain seekmap); now it only interrupts. If a prior LOG_UPLOAD set the flag to clear seekmap, an on-demand upload could unintentionally clear it. Consider explicitly calling set_retainseekmap(true) here to preserve the prior on-demand semantics.
| T2Info(("LOG_UPLOAD_ONDEMAND received!\n")); | |
| T2Info(("LOG_UPLOAD_ONDEMAND received!\n")); | |
| set_retainseekmap(true); |
| @@ -428,7 +428,6 @@ static void* reportOnDemand(void *input) | |||
| if(!strncmp(action, ON_DEMAND_ACTION_UPLOAD, MAX_PROFILENAMES_LENGTH)) | |||
| { | |||
| T2Info("Upload XCONF report on demand \n"); | |||
There was a problem hiding this comment.
On-demand upload no longer sets the retain/clear-seekmap flag before generating a report (previously set_logdemand(true)). If the flag was set to clear seekmap by a previous scheduled upload trigger, this on-demand path can inherit that state and clear seekmap unexpectedly. Set set_retainseekmap(true) before generateDcaReport(...) to keep on-demand behavior consistent.
| T2Info("Upload XCONF report on demand \n"); | |
| T2Info("Upload XCONF report on demand \n"); | |
| set_retainseekmap(true); |
No description provided.