Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device List: add "Remove disconnected devices" button #480

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

blammit
Copy link
Contributor

@blammit blammit commented Oct 20, 2023

The Device List should show devices that have been disconnected, i.e. those that have been removed from the source models as they are no longer valid.

So, instead of automatically syncing the aggregate model when a source model removes a device, keep the device in the aggregate model, and remove it when the removeDisconnectedDevices() functionality is triggered.

When a device is disconnected:

  • Show a cached copy of the device description, so that the list item text does not disappear.
  • Show "Not connected" as the secondary text; when the device is reconnected, the device status text should revert to whatever is appropriate for that device.

@blammit
Copy link
Contributor Author

blammit commented Oct 20, 2023

I tested this by:

  • disconnecting and reconnecting the battery on the cerbo
  • stopping and starting simulations in venus-docker

@blammit blammit force-pushed the device-list-disconnected branch 2 times, most recently from 5e446a6 to a337eb6 Compare October 23, 2023 00:32
@blammit blammit requested a review from chriadam October 23, 2023 02:43
This will allow the aggregate model to identify devices uniquely,
in the case where a device of the same service may appear in
multiple device models. For example, an inverter/charger may appear
in both Global.veBusDevices.model and Global.acInputs.model.
@blammit blammit changed the base branch from main to modelid October 23, 2023 03:13
Base automatically changed from modelid to main October 23, 2023 03:25
@blammit
Copy link
Contributor Author

blammit commented Oct 23, 2023

Fixed build issue on cerbo.

int index = indexOf(DeviceInfo::infoId(device, sourceModel));
if (index >= 0) {
// The device is already in the list, i.e. it was disconnected then reconnected.
QList<int> roles = QList<int>() << ConnectedRole;
Copy link
Contributor

@DanielMcInnes DanielMcInnes Oct 24, 2023

Choose a reason for hiding this comment

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

maybe: QList roles(ConnectedRole);
here and elsewhere

Copy link
Contributor

Choose a reason for hiding this comment

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

or roles { ConnectedRole };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

m_deviceInfos[index].device->disconnect(this);
}
m_deviceInfos[index].device.clear();
static const QList<int> roles = QList<int>() << ConnectedRole << DeviceRole;
Copy link
Contributor

@DanielMcInnes DanielMcInnes Oct 25, 2023

Choose a reason for hiding this comment

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

maybe: static const QList roles(ConnectedRole, DeviceRole);

Copy link
Contributor

Choose a reason for hiding this comment

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

or roles { ConnectedRole, DeviceRole };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

@DanielMcInnes DanielMcInnes left a comment

Choose a reason for hiding this comment

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

lgtm, nits

The Device List should show devices that have been disconnected, i.e.
those that have been removed from the source models as they are no
longer valid.

So, instead of automatically syncing the aggregate model when a source
model removes a device, keep the device in the aggregate model, and
remove it when the removeDisconnectedDevices() functionality is
triggered.

When a device is disconnected:
- Show a cached copy of the device description, so that the list item
text does not disappear.
- Show "Not connected" as the secondary text; when the device is
reconnected, the device status text should revert to whatever is
appropriate for that device.
@blammit blammit merged commit b57e65d into main Oct 25, 2023
1 check passed
@blammit blammit deleted the device-list-disconnected branch October 25, 2023 02:32
@blammit blammit restored the device-list-disconnected branch October 27, 2023 01:07
@blammit blammit deleted the device-list-disconnected branch October 27, 2023 01:08
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.

3 participants