-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Use device_id instead of config entry id and node id for zwave_js #12658
Conversation
The related core PR has been approved |
return; | ||
} | ||
showZWaveJSHealNodeDialog(this, { | ||
entry_id: this._entryId, | ||
node_id: this._nodeId, | ||
entry_id: this._entryId!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need entry_id
here, can't we also make that work with device id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the entry ID is still needed by dialog-zwave_js-heal-node
so that it can get the network status here:
frontend/src/panels/config/integrations/integration-panels/zwave_js/dialog-zwave_js-heal-node.ts
Lines 205 to 208 in bbe9dc6
const network: ZWaveJSNetwork = await fetchZwaveNetworkStatus( | |
this.hass!, | |
this.entry_id! | |
); |
dialog-zwave_js-heal-node
but we currently already have to figure out entry ID in ha-device-actions-zwave_js
so that we can add it to the URL, so why recalculate it? Is it necessary to add it to the URL though? I couldn't figure out why we do that, and if it's not necessary, we can just pass the device ID in and only calculate entry ID in the heal node dialog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another alternative would be to allow fetchZwaveNetworkStatus
to accept either an entry ID or a device ID since it's used in both contexts. This could easily be supported in the core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like the nicest approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to merge this and do that in a new PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea
...panels/config/devices/device-detail/integration-elements/zwave_js/ha-device-info-zwave_js.ts
Outdated
Show resolved
Hide resolved
...els/config/devices/device-detail/integration-elements/zwave_js/ha-device-actions-zwave_js.ts
Show resolved
Hide resolved
...panels/config/devices/device-detail/integration-elements/zwave_js/ha-device-info-zwave_js.ts
Show resolved
Hide resolved
…wave_js/ha-device-actions-zwave_js.ts Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>
…wave_js/ha-device-info-zwave_js.ts Co-authored-by: Paulus Schoutsen <paulus@home-assistant.io>
Proposed change
Instead of using config entry ID and node ID for any device specific WS commands, we now use device_id. This replaces #12642
Fixes #12641
Fixes #12136
Dependent on home-assistant/core#71667
Primary changes:
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: