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

feat: add connection identifiers to device-config #946

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jordvisser
Copy link

Description

Changes to the device-config-node and related files to make it possible to share device identifiers with Home Assistant.

Motivation and Context

As the IOT space keeps growing having the possibility to combine Node-Red entities with existing devices in Home Assistant is a needed option.

How Has This Been Tested?

  • Used the supplied dev-container for development and testing.
  • Ran npm run lint with no errors.
  • Ran npm run test with no errors.
  • Ran npm run dev with no errors.
  • Connected Node-Red to Home Assistant and got a working Entity with Device matched to already existing Device with the MAC address as the identifier
  • Connected Node-Red to Home Assistant and got a working Entity with Device with all new fields left empty.

Not Tested / Possibly missing

  • No tests done for the Bluetooth Identifier
  • No tests done for the UPnP Identifier
  • No tests done for the Zigbee Identifier
  • Did not check cookbook for needed updates
  • As I have no experience with the migrations.ts file(s) no work has been done on this part, don't know if this is needed.
  • Did not do any work with/on the device-node

@jordvisser
Copy link
Author

jordvisser commented May 24, 2023

I'm looking into the workings of the list field in the core change-node dialog ( https://github.com/node-red/node-red/blob/master/packages/node_modules/%40node-red/nodes/core/function/15-change.html ) as this would be a far more suitable way of displaying this feature in the edit dialog.
This would also make it an easy step to add the identifiers field for the device as well.

I have to admit that my typescript is a bit rusty, so this would take me a while before I will have it ready to be pulled.

At the moment the connections device list attribute is split into 4 variables in my pull request, looking at the ideal situation this should be a list in the device-config-node as well, please advise if the current pr is a good intermediate.
I'm leaning towards retracting the pr, but would like some guidance if the suggested feature warrant's going on a deep dive to make al list work in the device-config-node dialog.

@zachowj
Copy link
Owner

zachowj commented May 25, 2023

I have no objections to its implementation but lack knowledge regarding its potential impact on HA device functionality.

To optimize user experience, it may be beneficial to incorporate this feature into the entity configuration. This would offer users the ability to choose between a node-red generated device or an already established HA device via a drop-down menu, thereby eliminating the need for separate identification fields.

@jordvisser
Copy link
Author

jordvisser commented May 25, 2023

To my understanding it is the right way to have Node-Red make it's own device in HA, by using identifiers HA can then combine entities from different integrations in one view for the user. Making an 'identical' device should not create any problems in HA because HA expects that a device can become available via multiple components/integrations.
As explained visually in this image provided by HA ( Config Entry should be read as device for the purposes of this discussion ), source: HA developers docs

I do see added benefit to let the user select a existing HA device to automatically retrieve the identifiers and add these to the device-config-node.
This option could be placed at the very top of the device-config-node dialog, and implemented in such a way that NR will try to populate as much fields as possible with information retrieved from HA.
This auto-populate feature could be derived upon to make a auto-generate feature in the entity-config-node.
I think this should be a feature that helps the user in generating a device, instead of purely being a selection in a dropdown list that stays selected/activated, as the device generated by NR should be unique to the device(s) in HA.

I would like to suggest that I start the development in the device-config-node so that it can be developed in three parts: first the list fields, then then option to automatically populate the fields with information, followed by an implementation in the entity-config-node to automatically generate a device for the user.

@jordvisser jordvisser marked this pull request as draft May 25, 2023 06:52
@zachowj
Copy link
Owner

zachowj commented Jun 1, 2023

I would like to suggest that I start the development in the device-config-node so that it can be developed in three parts: first the list fields, then then option to automatically populate the fields with information, followed by an implementation in the entity-config-node to automatically generate a device for the user.

Sounds like a good plan.

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