Skip to content

descriptor: make dc_filter_suunto descriptor-specific for each EON Steel model - #142

Merged
mikeller merged 1 commit into
subsurface:Subsurface-DS9from
mikeller:feat/suunto-per-model-filter-170
Sep 30, 2026
Merged

mikeller merged 1 commit into
subsurface:Subsurface-DS9from
mikeller:feat/suunto-per-model-filter-170

Conversation

@mikeller

Copy link
Copy Markdown
Member

The filter previously ignored the descriptor parameter and checked USB HID
PID or BLE name prefix against the entire family's combined list. This
caused all four EON Steel descriptors (EON Steel, EON Core, D5, EON Steel
Black) to match any Suunto EONSTEEL family device, preventing automatic
model selection.

Refactor to index each flat array by dc_descriptor_get_model(descriptor)
so only the descriptor that corresponds to the connected device's USB PID
or BLE name prefix matches. Unknown model numbers return 0 (no match).
The two data arrays are left in the same form as upstream so that upstream
additions of new models merge cleanly.

This affects only the device selector presented to the user before a
download begins. The import process itself is unchanged: the Suunto EON
Steel driver echoes the descriptor model number back in DC_EVENT_DEVINFO,
so the coarse-model refinement path in the consumer never fires, and the
hw_id field is never populated, so the hw_id refinement path is also a
no-op. A user who selects the correct model manually sees no difference
in behaviour. The benefit is for automatic descriptor selection, which
now returns exactly one match instead of four.

…eel model

The filter previously ignored the descriptor parameter and checked USB HID
PID or BLE name prefix against the entire family's combined list.  This
caused all four EON Steel descriptors (EON Steel, EON Core, D5, EON Steel
Black) to match any Suunto EONSTEEL family device, preventing automatic
model selection.

Refactor to index each flat array by dc_descriptor_get_model(descriptor)
so only the descriptor that corresponds to the connected device's USB PID
or BLE name prefix matches.  Unknown model numbers return 0 (no match).
The two data arrays are left in the same form as upstream so that upstream
additions of new models merge cleanly.

This affects only the device selector presented to the user before a
download begins.  The import process itself is unchanged: the Suunto EON
Steel driver echoes the descriptor model number back in DC_EVENT_DEVINFO,
so the coarse-model refinement path in the consumer never fires, and the
hw_id field is never populated, so the hw_id refinement path is also a
no-op.  A user who selects the correct model manually sees no difference
in behaviour.  The benefit is for automatic descriptor selection, which
now returns exactly one match instead of four.

Signed-off-by: Michael Keller <github@ike.ch>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 00:08

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.

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 review overview

🟡 Changes recommended

EON Steel Black still also matches the shorter EON Steel BLE prefix.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread src/descriptor.c
return dc_match_usbhid (userdata, &usbhid[model]);
} else if (transport == DC_TRANSPORT_BLE) {
return DC_FILTER_INTERNAL (userdata, bluetooth, 0, dc_match_prefix);
return dc_match_prefix (userdata, &bluetooth[model]);
@mikeller

Copy link
Copy Markdown
Member Author

Thanks for the review. The BLE prefix ambiguity is technically real in isolation, but the concern does not apply to this PR.

dc_descriptor_filter is never called with DC_TRANSPORT_BLE inside libdivecomputer itself — there is no BLE device-name iterator in the library. Grepping all call sites confirms the function is invoked only via the USB HID, Bluetooth Classic, serial, IrDA, and USB iterators. The DC_TRANSPORT_BLE branch in dc_filter_suunto is unused by libdivecomputer's own device iteration.

Subsurface maps BLE device names to descriptors via its own prefix table in btdiscovery.cpp (getDeviceType()), which does not call dc_descriptor_filter at all. That table has a pre-existing gap (no "EON Steel Black" entry), but that is separate from this PR.

The PR also does not make the BLE situation worse than before: the old code passed all four BLE entries to dc_filter_internal, which would have matched "EON Steel Black" against entry 0 ("EON Steel") for every descriptor. The new code eliminates that cross-descriptor ambiguity for every entry including model 3 — model 0 retaining a prefix match on "EON Steel Black" is a pre-existing property of dc_match_prefix, unchanged by this PR.

The fix targets the USB HID path, which uses exact PID matching and is unambiguous after this change.

@mikeller
mikeller merged commit 736d0fe into subsurface:Subsurface-DS9 Sep 30, 2026
11 checks passed
@mikeller
mikeller deleted the feat/suunto-per-model-filter-170 branch September 30, 2026 10:35
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.

2 participants