Skip to content

Comments

feat(decoder): distinguish BLE and WiFi MAC addresses#178

Merged
michaelbeutler merged 3 commits intomainfrom
feat/distinguish-ble-wifi-mac-addresses
Feb 23, 2026
Merged

feat(decoder): distinguish BLE and WiFi MAC addresses#178
michaelbeutler merged 3 commits intomainfrom
feat/distinguish-ble-wifi-mac-addresses

Conversation

@michaelbeutler
Copy link
Contributor

@michaelbeutler michaelbeutler commented Feb 23, 2026

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

    • Added Bluetooth Low Energy (BLE) beacon detection for Port 3 payloads so devices can surface BLE beacon data.
  • Tests

    • Expanded test coverage to validate BLE beacon handling.
    • Improved test stability: AWS-dependent tests now skip when credentials/API are unavailable and a wrapper prevents panics in an example test.
  • Chores

    • Updated baseline metadata timestamp and related observed metadata.

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
Copilot AI review requested due to automatic review settings February 23, 2026 10:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 Beacon struct and UplinkFeatureBle interface 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.

@coderabbitai
Copy link

coderabbitai bot commented Feb 23, 2026

📝 Walkthrough

Walkthrough

The 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

Cohort / File(s) Summary
Core BLE Infrastructure
pkg/decoder/decoder.go
Adds Beacon struct (MAC, RSSI) and UplinkFeatureBle interface with GetBeacons() []Beacon.
Decoder Test Scaffolding
pkg/decoder/decoder_test.go, pkg/decoder/tagsl/v1/decoder_test.go
Adds unexported dummyBle test type implementing UplinkFeatureBle; extends tests to assert BLE feature presence and non-nil beacon collections.
Port 3 Feature Migration
pkg/decoder/tagsl/v1/port3.go, pkg/decoder/tagsl/v1/decoder.go
Changes port 3 configuration from FeatureWiFiFeatureBle; Port3Payload now implements UplinkFeatureBle; method GetAccessPoints() []AccessPointGetBeacons() []Beacon and internal variables/returns renamed accordingly.
Tests & Baseline Maintenance
.secrets.baseline, pkg/solver/aws/aws_test.go, examples/basic_tagxl/main_test.go
Updates secrets baseline timestamps/line numbers; adds t.Skip("requires AWS credentials") to AWS solver tests; wraps main() in a recover block in example test to skip on panic.

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)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🔵 New beacons sing where routers stood,
Port 3 turned to Bluetooth, understood,
Tests adjusted, baselines aligned,
MACs and RSSIs now defined,
A small bright change, neatly applied.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: distinguishing BLE from WiFi MAC addresses by introducing separate Beacon/UplinkFeatureBle types alongside existing AccessPoint/UplinkFeatureWiFi types.
Linked Issues check ✅ Passed All coding requirements from issue #160 are met: BLE scan uplinks (port 3) now implement UplinkFeatureBle instead of UplinkFeatureWiFi, and consumers can differentiate BLE from WiFi through separate type interfaces.
Out of Scope Changes check ✅ Passed Changes are focused and in-scope: baseline updates, BLE type additions, port 3 implementation switch to BLE, test coverage additions, and AWS test skips for credential requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/distinguish-ble-wifi-mac-addresses

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

🤖 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc6da2 and aa29d75.

📒 Files selected for processing (7)
  • .secrets.baseline
  • pkg/decoder/decoder.go
  • pkg/decoder/decoder_test.go
  • pkg/decoder/tagsl/v1/decoder.go
  • pkg/decoder/tagsl/v1/decoder_test.go
  • pkg/decoder/tagsl/v1/port3.go
  • pkg/solver/aws/aws_test.go

@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Feb 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
pkg/decoder/decoder.go 100.00% <ø> (ø)
pkg/decoder/tagsl/v1/decoder.go 100.00% <100.00%> (ø)
pkg/decoder/tagsl/v1/port3.go 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

🤖 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.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa29d75 and 753e95c.

📒 Files selected for processing (1)
  • examples/basic_tagxl/main_test.go

@michaelbeutler michaelbeutler requested a review from a team February 23, 2026 10:58
@michaelbeutler michaelbeutler merged commit aeac12a into main Feb 23, 2026
10 checks passed
@michaelbeutler michaelbeutler deleted the feat/distinguish-ble-wifi-mac-addresses branch February 23, 2026 11:18
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.

BLE uplink implements WiFi features

2 participants