Fix adapter device ID not incrementing when adding new devices

[jgeudens]

When adding a new device via AdapterDeviceSettings, the new tab always received the adapter's hardcoded default id (e.g. 1) instead of a unique one, allowing multiple devices to share the same id. No corresponding SettingsModel device was created either, breaking the invariant that adapter-config device ids must match SettingsModel device ids.

Fix handleAddTab() to call addNewDevice() for a unique id and set the adapter on the resulting SettingsModel entry. Also fix onAdapterChanged() in DeviceConfigTab to preserve the existing device id when the user switches the protocol adapter, preventing the unique id from being overwritten by the new adapter's defaults.

https://claude.ai/code/session_012tQA6XhU6TKDgqzUwzJqx1

Summary by CodeRabbit

• Bug Fixes

  • Device identifiers are preserved when switching an adapter in a device configuration tab.
  • Adding a new device tab now creates and registers a distinct device with a unique assigned ID, avoiding reuse of adapter defaults.

• Tests

  • Added and enhanced tests to verify device ID assignment, uniqueness across added tabs, and ID preservation when adapters change.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1cb1b9f4-4711-43da-b88a-c5354a0c45e3

📥 Commits

Reviewing files that changed from the base of the PR and between 8c86c51 and 541a464.

📒 Files selected for processing (2)

• src/customwidgets/deviceconfigtab.cpp
• src/customwidgets/deviceconfigtab.h

🚧 Files skipped from review as they are similar to previous changes (1)

• src/customwidgets/deviceconfigtab.cpp

---

Walkthrough

Device tab creation now allocates and registers a model-backed device ID; adapter switches preserve an existing device "id" by injecting it into the schema form's default values before rebuilding the form.

Changes

Cohort / File(s)	Summary
DeviceConfigTab (preserve id on adapter change) src/customwidgets/deviceconfigtab.h, src/customwidgets/deviceconfigtab.cpp	Adds private _deviceId and, on adapter change, reads the current schema form values (if present) or _deviceId and injects that into defaultValues["id"] before calling rebuildSchemaForm(...), preserving the device id.
AdapterDeviceSettings (allocate device on Add Tab) src/dialogs/adapterdevicesettings.cpp	Calls _pSettingsModel->addNewDevice() to allocate a deviceId_t, sets its adapter via deviceSettings(newId)->setAdapterId(...), and pre-populates the new tab's defaultValues["id"] with the assigned id.
Tests: updated and added cases tests/dialogs/tst_adapterdevicesettings.cpp, tests/dialogs/tst_adapterdevicesettings.h	Updates existing test expectations to accept model-assigned ids; adds tests addTabIncrementsDeviceId() and deviceIdPreservedWhenAdapterChanged() and declares their slots to verify id allocation, uniqueness/incrementing, and preservation on adapter change.

Sequence Diagram(s)

sequenceDiagram
    rect rgba(220,220,255,0.5)
    participant User
    participant Dialog as AdapterDeviceSettings
    participant Model as SettingsModel
    participant Tab as DeviceConfigTab
    participant Schema as SchemaForm
    end

    Note over User,Dialog: Flow A — Add new device tab
    User->>Dialog: click "Add Tab"
    Dialog->>Model: addNewDevice()
    Model-->>Dialog: newDeviceId
    Dialog->>Model: deviceSettings(newDeviceId).setAdapterId(defaultAdapterId)
    Dialog->>Tab: create tab (defaultValues with id = newDeviceId)
    Tab->>Schema: rebuildSchemaForm(adapterId, defaultValues)
    Schema-->>Tab: populated form (id = newDeviceId)

    Note over User,Tab: Flow B — Change adapter in existing tab
    User->>Tab: select different adapter
    Tab->>Tab: read currentValues["id"] (or use _deviceId)
    Tab->>Tab: inject id into defaultValues["id"]
    Tab->>Schema: rebuildSchemaForm(newAdapterId, defaultValues)
    Schema-->>Tab: populated form (id preserved)

Loading

Possibly related PRs

• Dev/UI type dependant #15 — Modifies the same adapter/default-value derivation paths in DeviceConfigTab and AdapterDeviceSettings; likely directly related.
• Configuration (UI) #6 — Earlier changes to DeviceConfigTab/AdapterDeviceSettings default-value handling that this PR further refines.
• UI names #13 — Alters device id and tab construction logic in the same UI code paths.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly describes the main fix: preventing device ID loss when adding new devices via the adapter device settings interface.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/fix-device-id-increment-uJayt

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jgeudens]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

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 `@tests/dialogs/tst_adapterdevicesettings.cpp`:
- Around line 239-270: Add a regression test that exercises the "ID must remain
unchanged when switching adapter" path in DeviceConfigTab::onAdapterChanged:
extend or add a test (e.g., alongside
TestAdapterDeviceSettings::addTabIncrementsDeviceId) that creates a
SettingsModel, sets up two adapters (e.g., "adapterA" and "adapterB") via
setupAdapter, adds a DeviceConfigTab through
AdapterDeviceSettings/AddableTabWidget, captures the assigned device id from
tab->values()["id"], then simulate changing the tab's adapter to the other
adapter (invoke the same adapter-change action used by
DeviceConfigTab::onAdapterChanged) and assert the tab's id remains equal to the
original id, the model still has that deviceId, and
deviceSettings(...)->adapterId() reflects the new adapter; reference
DeviceConfigTab, AdapterDeviceSettings, AddableTabWidget, SettingsModel and
TestAdapterDeviceSettings when locating where to add the test.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 65e8170b-b793-442e-a28e-4c961563399e

📥 Commits

Reviewing files that changed from the base of the PR and between 705c8f3 and 6c41b0e.

📒 Files selected for processing (4)

• src/customwidgets/deviceconfigtab.cpp
• src/dialogs/adapterdevicesettings.cpp
• tests/dialogs/tst_adapterdevicesettings.cpp
• tests/dialogs/tst_adapterdevicesettings.h

[coderabbitai]

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 `@src/customwidgets/deviceconfigtab.cpp`:
- Around line 90-96: The code reads id from _pSchemaForm->values() and skips
setting defaultValues["id"] when id is hidden, allowing adapter defaults to
overwrite and produce duplicate IDs; initialize and store a member _deviceId
from the constructor input deviceValues["id"] (or -1 if absent) and then in the
block that currently checks _pSchemaForm and currentId, fall back to _deviceId
when values().value("id") is missing/invalid so defaultValues["id"] is always
populated with the original device ID; update the constructor to accept and set
_deviceId and update the code around _pSchemaForm->values() to prefer form id
then _deviceId.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5a633c65-4c4d-4210-a82b-f00fd5bd923d

📥 Commits

Reviewing files that changed from the base of the PR and between 6c41b0e and 8c86c51.

📒 Files selected for processing (4)

• src/customwidgets/deviceconfigtab.cpp
• src/dialogs/adapterdevicesettings.cpp
• tests/dialogs/tst_adapterdevicesettings.cpp
• tests/dialogs/tst_adapterdevicesettings.h

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/dialogs/tst_adapterdevicesettings.h
• tests/dialogs/tst_adapterdevicesettings.cpp