feat(ble): add BLE ports 160-163 to smartlabel and tagxl decoders#192
Conversation
Decode the ts2 BLE uplink payloads (RSSI + MAC beacon lists) on ports 160 (steady), 161 (moving), 162 (steady + timestamp) and 163 (moving + timestamp) for both the smartlabel and tagxl v1 decoders. Each payload exposes the Ble and Moving features (plus Timestamp/Buffered for the timestamped variants), mirroring the existing Wi-Fi port layout. Also refresh .secrets.baseline for the new test payload vectors.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds BLE uplink decoder support for ports 160–163 to both the SmartLabel v1 and TagXL v1 decoders. Four new payload types per decoder handle steady/moving variants (ports 160/161) and timestamped steady/moving variants (ports 162/163), with ChangesBLE Ports 160–163 (SmartLabel + TagXL)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds decoding support for TS2 BLE uplinks (RSSI + MAC beacon lists) on ports 160–163 for both SmartLabel v1 and TagXL v1, aligning the port layout with the existing steady/moving + (optional) timestamp variants and expanding test coverage for these new payload vectors.
Changes:
- Add new BLE payload types and beacon extraction for ports 160–163 (steady/moving; with/without timestamp) in SmartLabel v1 and TagXL v1 decoders.
- Extend both decoder configs to map the new port layouts into the new payload structs and expose the corresponding decoder features.
- Add decode + feature tests for new payload vectors and refresh
.secrets.baselineaccordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/decoder/tagxl/v1/port160.go | New TagXL v1 BLE steady (no timestamp) payload type + beacon extraction |
| pkg/decoder/tagxl/v1/port161.go | New TagXL v1 BLE moving (no timestamp) payload type + beacon extraction |
| pkg/decoder/tagxl/v1/port162.go | New TagXL v1 BLE steady (timestamped) payload type + beacon extraction + buffered semantics |
| pkg/decoder/tagxl/v1/port163.go | New TagXL v1 BLE moving (timestamped) payload type + beacon extraction + buffered semantics |
| pkg/decoder/tagxl/v1/decoder.go | Adds port-to-payload field mappings + feature exposure for ports 160–163 |
| pkg/decoder/tagxl/v1/decoder_test.go | Adds decode + feature tests for new BLE ports (160–163) |
| pkg/decoder/smartlabel/v1/port160.go | New SmartLabel v1 BLE steady (no timestamp) payload type + beacon extraction |
| pkg/decoder/smartlabel/v1/port161.go | New SmartLabel v1 BLE moving (no timestamp) payload type + beacon extraction |
| pkg/decoder/smartlabel/v1/port162.go | New SmartLabel v1 BLE steady (timestamped) payload type + beacon extraction |
| pkg/decoder/smartlabel/v1/port163.go | New SmartLabel v1 BLE moving (timestamped) payload type + beacon extraction |
| pkg/decoder/smartlabel/v1/decoder.go | Adds port-to-payload field mappings + feature exposure for ports 160–163 |
| pkg/decoder/smartlabel/v1/decoder_test.go | Adds decode + feature tests for new BLE ports (160–163) |
| .secrets.baseline | Updates detect-secrets baseline for added high-entropy test vectors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/decoder/smartlabel/v1/decoder_test.go`:
- Around line 783-790: Replace the weak nil check for GetBeacons() with a
stronger assertion that validates the beacons slice actually contains data. In
the test block where you check if ble.GetBeacons() == nil, change this to
instead check that len(ble.GetBeacons()) > 0 to ensure beacon count is greater
than zero and catch potential decode regressions where beacons might be empty.
In `@pkg/decoder/smartlabel/v1/port160.go`:
- Around line 50-90: The beacon inclusion logic currently requires both MAC and
RSSI to be present before adding beacons, which can silently drop MAC addresses
when RSSI is unavailable and diverges from the existing BLE beacon contract.
Change the inclusion logic in the beacon appending blocks to key only on MAC
presence: for Mac1, check only if p.Mac1 is not empty (remove the Rssi1 check);
for Mac2 through Mac6, check only if the Mac pointer is non-nil (remove the Rssi
checks). When appending beacons, always include the RSSI pointer as-is, allowing
it to be nil when unavailable instead of dropping valid MAC entries.
In `@pkg/decoder/tagxl/v1/decoder_test.go`:
- Around line 1571-1578: The assertion in the BLE feature test using
GetBeacons() == nil check is insufficient because it will pass even when
GetBeacons() returns an initialized empty slice, missing potential BLE decoding
regressions. Replace the nil check with a stronger assertion that verifies
GetBeacons() actually contains the expected beacon data, such as checking that
the slice has a non-zero length or contains the specific beacons expected from
the decoded payload in this test case.
In `@pkg/decoder/tagxl/v1/port160.go`:
- Around line 57-90: The code appends optional beacons when the MAC pointer is
not nil (p.Mac2, p.Mac3, p.Mac4, p.Mac5, p.Mac6) but does not validate that the
corresponding RSSI pointer is also not nil, which can result in partial beacon
entries being emitted. Modify each if condition in the repeated beacon appending
blocks to require both the MAC pointer AND the RSSI pointer to be not nil before
appending to the beacons slice. This ensures complete MAC/RSSI pairs are
validated before creating and appending each decoder.Beacon entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11ea685f-5cf3-4a06-95e9-69424fc8ebd9
📒 Files selected for processing (13)
.secrets.baselinepkg/decoder/smartlabel/v1/decoder.gopkg/decoder/smartlabel/v1/decoder_test.gopkg/decoder/smartlabel/v1/port160.gopkg/decoder/smartlabel/v1/port161.gopkg/decoder/smartlabel/v1/port162.gopkg/decoder/smartlabel/v1/port163.gopkg/decoder/tagxl/v1/decoder.gopkg/decoder/tagxl/v1/decoder_test.gopkg/decoder/tagxl/v1/port160.gopkg/decoder/tagxl/v1/port161.gopkg/decoder/tagxl/v1/port162.gopkg/decoder/tagxl/v1/port163.go
Strengthen the FeatureBle assertions in the smartlabel and tagxl feature tests to require at least one decoded beacon instead of a non-nil slice, catching empty-decode regressions.
Decode the ts2 BLE uplink payloads (RSSI + MAC beacon lists) on ports 160 (steady), 161 (moving), 162 (steady + timestamp) and 163 (moving + timestamp) for both the smartlabel and tagxl v1 decoders.
Each payload exposes the Ble and Moving features. The timestamped variants (162/163) additionally expose Timestamp; the tagxl variants also expose Buffered, mirroring its existing timestamped Wi-Fi ports. The smartlabel timestamped variants intentionally omit Buffered, consistent with its other timestamped ports (e.g. port 10).
Also refresh .secrets.baseline for the new test payload vectors.
Summary by CodeRabbit
Release Notes
New Features
Tests