Skip to content

app_rpt: Refactor telem_done#868

Merged
mkmer merged 1 commit intomasterfrom
telem_done_2
Dec 14, 2025
Merged

app_rpt: Refactor telem_done#868
mkmer merged 1 commit intomasterfrom
telem_done_2

Conversation

@mkmer
Copy link
Collaborator

@mkmer mkmer commented Dec 14, 2025

Turns out it is "normal" to play a telemetry message that is NOT active, but it's not "normal" to clear active_telem if not the matching thread.
This PR addresses a "normal" case warning message that is not important.
See #865 (comment)
Created here: #852

Summary by CodeRabbit

  • Chores
    • Enhanced internal telemetry diagnostics with improved logging for better error tracking and system monitoring.

✏️ Tip: You can customize this high-level summary in your review settings.

Turns out it is "normal" to play a telemetry message that is NOT active, but it's not "normal" to clear active_telem if not the matching thread.
This PR addresses a "normal" case warning message that is not important.
@mkmer mkmer self-assigned this Dec 14, 2025
@mkmer mkmer added the bug Something isn't working label Dec 14, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 14, 2025

📝 Walkthrough

Walkthrough

The telem_done macro in apps/app_rpt/rpt_telemetry.c is modified to add debug logging and implement a pre-check that validates active_telem before clearing it, logging a warning when the active telemetry does not match the current telemetry context.

Changes

Cohort / File(s) Change Summary
Telemetry macro logging and validation
apps/app_rpt/rpt_telemetry.c
Modified telem_done macro to add early debug log, introduce pre-check for active_telem mismatch with warning log, and adjust clearing logic to only clear active_telem when it matches the current telem

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review the conditional logic change to ensure active_telem clearing behavior is correct
  • Verify the new logging statements provide adequate diagnostic information
  • Confirm that the removal of the previous else-branch logging doesn't lose important error tracking

Possibly related PRs

Suggested labels

code quality

Suggested reviewers

  • Allan-N

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'app_rpt: Refactor telem_done' is directly related to the main change—it refers to the refactoring of the telem_done macro described in the changeset.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch telem_done_2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 414e040 and 0cc71ba.

📒 Files selected for processing (1)
  • apps/app_rpt/rpt_telemetry.c (1 hunks)
⏰ Context from checks skipped due to timeout of 100000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: container

@mkmer mkmer merged commit e698f4f into master Dec 14, 2025
4 checks passed
@mkmer mkmer deleted the telem_done_2 branch December 14, 2025 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants