Skip to content

fix(microservices): drop pre-identified tlm for unmapped targets - #4080

Merged
ryanmelt merged 1 commit into
fix/interface-tlm-identificationfrom
fix/interface-tlm-unmapped-drop
Oct 21, 2026
Merged

ryanmelt merged 1 commit into
fix/interface-tlm-identificationfrom
fix/interface-tlm-unmapped-drop

Conversation

@jmthomas

Copy link
Copy Markdown
Member

Summary

Stacked on #4000. Changes how InterfaceMicroservice#handle_packet (Ruby and Python) handles a pre-identified packet whose target isn't mapped to the interface.

In #4000 such a packet has its identity cleared and is identified by content against the mapped targets. That has two problems:

  • It is usually published as UNKNOWN UNKNOWN. UNKNOWN is always in tlm_target_enabled, so these packets now reach the stream, where on main they were dropped.
  • It can be published under the wrong name. If a mapped packet's ID items happen to match the bytes, the data is decommutated with that definition. A mapped packet with no ID items matches every buffer (Packet#identify?).

On main, unmapped pre-identified packets were already dropped silently by the tlm_target_enabled check before TelemetryTopic.write_packet. This PR keeps that outcome but makes it explicit:

  • Unmapped target: keep the header identity and drop the packet. Log Dropping telemetry for unmapped target: TGT PKT once per target/packet pair, so replaying a multi-target COSMOS 4 log doesn't flood the log.
  • Mapped target, unknown packet name: unchanged from fix(microservices): validate pre-identified telemetry and keep stored data out of CVT #4000. Warn and fall back to content identification.
  • Docs: the Preidentified Protocol section in protocols.md now says every target in the packet headers must be mapped with MAP_TARGET / MAP_TLM_TARGET.

Since unmapped packets end up dropped as before, no upgrade note is needed.

Test plan

  • bundle exec rspec spec/microservices/interface_microservice_spec.rb spec/topics (76 examples, 0 failures)
  • uv run pytest test/microservices/test_interface_microservice.py test/topics (72 passed)
  • The new "drops ... unmapped ... warns once" test fails without the source change, in both Ruby and Python
  • ruff check / ruff format --check

🤖 Generated with Claude Code

- Keep the header identity and drop packets whose target is not in
  tlm_target_names, instead of re-identifying them by content, which
  could publish them under a mapped packet's name
- Warn once per target/packet via @unmapped_tlm_warned so replaying
  multi-target logs does not flood the log
- Document the mapping requirement in the Preidentified Protocol docs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 21:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8062fcae-31c3-4f57-afa9-59013659f1a4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.16%. Comparing base (4988cb4) to head (247a169).

Additional details and impacted files
@@                         Coverage Diff                          @@
##           fix/interface-tlm-identification    #4080      +/-   ##
====================================================================
- Coverage                             80.16%   80.16%   -0.01%     
====================================================================
  Files                                   901      901              
  Lines                                 68449    68428      -21     
  Branches                               2714     2714              
====================================================================
- Hits                                  54874    54853      -21     
  Misses                                12906    12906              
  Partials                                669      669              
Flag Coverage Δ
python 80.23% <ø> (+0.06%) ⬆️
ruby-api 82.30% <ø> (-0.37%) ⬇️
ruby-backend 85.70% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jmthomas
jmthomas requested a review from ryanmelt October 21, 2026 17:52
@ryanmelt
ryanmelt merged commit 7c946d4 into fix/interface-tlm-identification Oct 21, 2026
35 of 40 checks passed
@ryanmelt
ryanmelt deleted the fix/interface-tlm-unmapped-drop branch October 21, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants