feat(decoder): distinguish BLE and WiFi MAC addresses#178
feat(decoder): distinguish BLE and WiFi MAC addresses#178michaelbeutler merged 3 commits intomainfrom
Conversation
Add Beacon struct and UplinkFeatureBle interface so consumers can differentiate BLE scan uplinks (port 3) from WiFi access point uplinks. Port 3 in tagsl/v1 now correctly implements UplinkFeatureBle instead of UplinkFeatureWiFi. Closes #160
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #160 by correctly distinguishing BLE scan uplinks from WiFi access point uplinks in the tagsl/v1 decoder. Previously, port 3 (BLE scan uplink) incorrectly implemented the UplinkFeatureWiFi interface; it now properly implements a new UplinkFeatureBle interface.
Changes:
- Added
Beaconstruct andUplinkFeatureBleinterface to the decoder package - Updated port 3 implementation to use BLE features instead of WiFi features
- Added comprehensive test coverage for the BLE feature
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/decoder/decoder.go | Defines new Beacon struct and UplinkFeatureBle interface for BLE beacon data |
| pkg/decoder/tagsl/v1/port3.go | Changes port 3 from implementing UplinkFeatureWiFi to UplinkFeatureBle, renaming method from GetAccessPoints() to GetBeacons() |
| pkg/decoder/tagsl/v1/decoder.go | Updates port 3 feature flag from FeatureWiFi to FeatureBle |
| pkg/decoder/tagsl/v1/decoder_test.go | Adds test coverage for the new BLE feature |
| pkg/decoder/decoder_test.go | Adds dummy BLE implementation for testing |
| .secrets.baseline | Updates line numbers and timestamp due to test file changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThe pull request adds BLE support: a new Beacon type and UplinkFeatureBle interface, migrates Port 3 payload handling from WiFi to BLE (GetAccessPoints → GetBeacons), updates tests and test scaffolding, and updates secrets baseline and some AWS-related tests/configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Payload as "Uplink Payload"
participant Decoder as "tagsl/v1 Decoder"
participant Port3 as "Port3Payload"
participant Consumer as "Feature Consumer"
Payload->>Decoder: receive port 3 data
Decoder->>Port3: parse into Port3Payload
Port3->>Port3: populate Beacons []Beacon
Port3->>Consumer: expose via UplinkFeatureBle.GetBeacons()
Consumer->>Port3: iterate beacons (MAC, RSSI)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/solver/aws/aws_test.go`:
- Around line 16-18: The test currently unconditionally skips in TestSolve;
change it to conditionally skip based on the presence of AWS credentials by
adding a short helper (e.g., hasAWSCreds or requireAWSCreds) and invoking it at
the start of TestSolve (and the similar test around lines 120-122) so the test
only skips when creds are missing. The helper should check known signals such as
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY (and AWS_SESSION_TOKEN if needed) or
attempt to create a minimal AWS session, and call t.Skipf with a clear message
when no credentials are found; update TestSolve to call that helper instead of
t.Skip to allow CI/dev runs with valid creds.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.secrets.baselinepkg/decoder/decoder.gopkg/decoder/decoder_test.gopkg/decoder/tagsl/v1/decoder.gopkg/decoder/tagsl/v1/decoder_test.gopkg/decoder/tagsl/v1/port3.gopkg/solver/aws/aws_test.go
|
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/basic_tagxl/main_test.go`:
- Around line 15-19: The deferred recover in the test currently skips on any
panic which hides real failures; update the deferred anonymous function (the
defer func that recovers around main()) to inspect r and only call t.Skipf when
r indicates a known AWS-related panic (e.g., check if r is an error or string
and contains AWS-specific signals like "NoCredentialProviders",
"EC2MetadataError", "RequestCanceled", "unable to locate credentials", or
another project-specific AWS panic marker); if it does not match those AWS
indicators, re-raise the panic (panic(r)) so real regressions surface.



Add Beacon struct and UplinkFeatureBle interface so consumers can differentiate BLE scan uplinks (port 3) from WiFi access point uplinks. Port 3 in tagsl/v1 now correctly implements UplinkFeatureBle instead of UplinkFeatureWiFi.
Closes #160
Summary by CodeRabbit
New Features
Tests
Chores