Skip to content

feat(tagsl): compare reflected configuration - #202

Merged
niko-kriznik-globtim merged 5 commits into
mainfrom
feat/tagsl-configuration-match
Sep 1, 2026
Merged

niko-kriznik-globtim merged 5 commits into
mainfrom
feat/tagsl-configuration-match

Conversation

@niko-kriznik-globtim

@niko-kriznik-globtim niko-kriznik-globtim commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • expose decoder-owned Tag S/L configuration downlink/report ports and semantic matching of port-128 downlinks against port-4 reports
  • compare optional batch/buffer only when the report includes those tails, so a valid 28-byte config report can still verify
  • match BLE scan/advertising config: port-134 downlinks against the same 22-byte layout on port-8 reports
  • keep Tag XL setter and report TLV tag codes next to the port 151 decoder so matching and decoding share one source of truth

Supports truvami/api#972.

Test plan

  • go test ./pkg/decoder/tagxl/v1 ./pkg/decoder/tagsl/v1 ./pkg/common ./pkg/encoder/tagsl/v1 -count=1
  • pre-commit and commit-message hooks

Keep setter and report TLV tags next to the port 151 decoder so matching and decoding cannot drift.
Verify Tag S/L campaigns by matching port-128 downlinks against the comparable fields in port-4 reports.
@niko-kriznik-globtim

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Essentials

Run ID: b2f20541-5aa7-410f-bbd9-f4d4426949f0

📥 Commits

Reviewing files that changed from the base of the PR and between 0ac5514 and 6c78f4a.

📒 Files selected for processing (4)
  • pkg/decoder/tagsl/v1/configuration_match.go
  • pkg/decoder/tagsl/v1/configuration_match_test.go
  • pkg/decoder/tagsl/v1/decoder.go
  • pkg/decoder/tagsl/v1/port8.go

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

Adds TAGSL configuration matching for standard and BLE payloads. It validates payload lengths, decodes shared payload layouts, compares configuration fields, and supports optional report fields. It also centralizes TAGXL configuration mappings and TLV matching.

Changes

Configuration matching and payload mappings

Layer / File(s) Summary
TAGSL payload contracts
pkg/decoder/tagsl/v1/port128.go, pkg/decoder/tagsl/v1/port4.go, pkg/decoder/tagsl/v1/port8.go, pkg/decoder/tagsl/v1/decoder.go, pkg/encoder/tagsl/v1/port128.go, pkg/encoder/tagsl/v1/encoder.go
Defines shared TAGSL payload types and binary layouts. Decoder and encoder paths reuse the shared configurations.
TAGSL configuration matching
pkg/decoder/tagsl/v1/configuration_match.go, pkg/decoder/tagsl/v1/configuration_match_test.go
Adds standard and BLE hexadecimal payload matching, length validation, field comparison, optional batch and buffer handling, and coverage for valid and invalid inputs.
TAGXL configuration tag mappings
pkg/decoder/tagxl/v1/port151.go, pkg/decoder/tagxl/v1/configuration_match.go, pkg/decoder/tagxl/v1/decoder.go, pkg/decoder/tagxl/v1/configuration_match_test.go
Adds named configuration identifiers and shared setter-to-report mappings. Matching uses normalized TLVs, and tests verify comparable tag coverage.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 6c78f

This change adds reflected Tag S/L configuration matching and preserves verification for reports with optional fields; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant MatchConfiguration
  participant TAGSLDecoder
  Caller->>MatchConfiguration: provide sentHex and observedHex
  MatchConfiguration->>TAGSLDecoder: decode and validate payloads
  TAGSLDecoder-->>MatchConfiguration: return validated configuration values
  MatchConfiguration-->>Caller: return match result or error
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.34% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 12 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding Tag S/L configuration comparison for reflected configurations.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/tagsl-configuration-match

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

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@codecov-commenter

codecov-commenter commented Sep 1, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 96.80851% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/decoder/tagsl/v1/configuration_match.go 92.50% 3 Missing and 3 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Files with missing lines Coverage Δ
pkg/decoder/tagsl/v1/decoder.go 100.00% <100.00%> (ø)
pkg/decoder/tagsl/v1/port128.go 100.00% <100.00%> (ø)
pkg/decoder/tagsl/v1/port4.go 100.00% <100.00%> (ø)
pkg/decoder/tagsl/v1/port8.go 100.00% <100.00%> (ø)
pkg/decoder/tagxl/v1/configuration_match.go 89.83% <100.00%> (-0.42%) ⬇️
pkg/decoder/tagxl/v1/decoder.go 91.48% <100.00%> (ø)
pkg/decoder/tagxl/v1/port151.go 92.51% <100.00%> (+0.26%) ⬆️
pkg/encoder/tagsl/v1/encoder.go 100.00% <100.00%> (ø)
pkg/decoder/tagsl/v1/configuration_match.go 92.50% <92.50%> (ø)
🚀 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.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 1, 2026
Call the port 151 identifiers TLV tags so the decoder schema is not framed as a report.
Own the port-128 layout in the decoder so encoding and campaign matching share one struct.
@niko-kriznik-globtim

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 1, 2026
@niko-kriznik-globtim
niko-kriznik-globtim marked this pull request as ready for review September 1, 2026 08:34
michaelbeutler
michaelbeutler previously approved these changes Sep 1, 2026
@michaelbeutler

Copy link
Copy Markdown
Contributor

@niko-kriznik-globtim please be aware of the BLE, DataRate and Accuracy Booster Downlinks. There is way of verification but might still be used in the configuration campaigns.

Port 134 downlinks are reported on port 8 with the same 22-byte layout, so campaigns can verify BLE scan config the same way as port 128 vs port 4.
@niko-kriznik-globtim

Copy link
Copy Markdown
Contributor Author

@michaelbeutler thanks — we took the BLE path.

Port 134 BLE scan/advertising config is reported on port 8 (same 22-byte layout), so this PR now has MatchBleConfiguration for that pair. API #972 will verify Tag S/L campaigns on port 134 against those port-8 reports.

DataRate is not a Tag S/L downlink in this protocol (Smart Label port 128/4 and Tag XL TLV only). Accuracy booster (port 131) has no reflecting uplink, so those campaigns stay unverified, same as today.

Port 128 BLE/GNSS/Wi-Fi enable flags are still ignored: they are not on the port-4 config report.

@niko-kriznik-globtim

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@niko-kriznik-globtim
niko-kriznik-globtim merged commit 2a9ec2e into main Sep 1, 2026
8 checks passed
@niko-kriznik-globtim
niko-kriznik-globtim deleted the feat/tagsl-configuration-match branch September 1, 2026 11:30
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