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

Multi-IoTaWatt setup issue with identical inputs #14

Closed
janecker opened this issue Aug 14, 2021 · 13 comments
Closed

Multi-IoTaWatt setup issue with identical inputs #14

janecker opened this issue Aug 14, 2021 · 13 comments

Comments

@janecker
Copy link

Hi,

first of all thanks for your work, I am new to HA and I am currently testing your integration. I have 3 IoTaWatt units in use (one for each phase). With that setup I have notices an issue with your integration:

If I have two inputs with the same name in multiple IoTaWatt units, the entities cannot be created properly. For me this is the case for the voltage, which is named the same in each and every unit. Maybe it would be a good idea to add a prefix to every entitiy name tha represents the IoTaWatt unit to which it belongs.

@gtdiehl
Copy link
Owner

gtdiehl commented Aug 27, 2021

@janecker I knew this would become an issue at some point! The name is coming from the iotawattpy library. Since the library does not know about each hardware instance, but Home Assistant does maybe the name() method for the IoTawatt HA integration code could insert the name that is entered through the Config Flow UI into the entity name. Does Home Assistant prevent duplicate names? Maybe would have to be checked during the Config Flow setup?

If we could get to unique names entered through the Config Flow UI, then for example if you enter in phase1 during setup for the name, the iotawattpy getName() API method would return iotawatt_output_pool then the Home Assistant name() could change the name to something like iotawatt_phase1_output_pool.

@janecker
Copy link
Author

@gtdiehl: Thanks for the response, your idea with the naimg was exactly what my thought was. The only issue witht this might be, that it will break the existing names and will potentially break a lot of stuff if someone updates to the newer version.

I don't think HA does allow multiple entites with the same unique ID (same display name is fine). I think I have seen other integraitons just adding _1, _2 etc. as a suffix to the unique IDs if they already exist, but I am not sure how it is possible to assign these unique IDs to the correct Iotwatt instance than...

@gtdiehl
Copy link
Owner

gtdiehl commented Aug 30, 2021

@janecker I see your point with updating to a new version, but with the integration being bundled with Home Assistant unfortunately this will be the case. The integration, right now the way the code stands from the review process, is that the sensor names have been reduced in length. The iotawatt and I/O type have been dropped keeping only the sensor name.

HA will automatically append _1 or whatever the next integer value is available to not create duplicates. Also, the Input sensors (again in the integration code) have unique IDs so the user can rename them from within the HA UI. Output sensors will need to have a unique ID. I know in the custom integration unique IDs are being created for Output sensors, but right now the way the code is Outputs do not have a unique ID so they can't be renamed. Hopefully, tThis Output unique ID issue will should resolved with an updated IoTaWatt firmware before the first release.

But getting back to the example you had in the original issue I'm wondering is this a display issue or does Home Assistant have an issue showing the second Voltage sensor? The custom integration right now should be setting a unique ID, unless some bug in the code. When discovering the second iotawatt you should get Voltage_1. Is this what you currently see? Or is there some error in the log saying the sensor already has a unique ID, so it is ignored?

@janecker
Copy link
Author

@janecker I see your point with updating to a new version, but with the integration being bundled with Home Assistant unfortunately this will be the case. The integration, right now the way the code stands from the review process, is that the sensor names have been reduced in length. The iotawatt and I/O type have been dropped keeping only the sensor name.

For me this is not to big of an issue, I just have to make sure to rename some template entites when updating to the newer version, just wanted to mention it :).

But getting back to the example you had in the original issue I'm wondering is this a display issue or does Home Assistant have an issue showing the second Voltage sensor? The custom integration right now should be setting a unique ID, unless some bug in the code. When discovering the second iotawatt you should get Voltage_1. Is this what you currently see? Or is there some error in the log saying the sensor already has a unique ID, so it is ignored?

I just checked my logs and I am getting the following errors regarding the addtional voltage inputs:

2021-08-19 21:59:11 ERROR (MainThread) [homeassistant.components.sensor] Platform iotawatt does not generate unique IDs. ID _Input_Voltage already exists - ignoring sensor.iotawatt_input_voltage
2021-08-19 21:59:11 ERROR (MainThread) [homeassistant.components.sensor] Platform iotawatt does not generate unique IDs. ID _Input_Voltage already exists - ignoring sensor.iotawatt_input_voltage

There is only one entity showing up as sensor.iotawatt_input_voltage.

While checking the log I also noticed the following error quite a few times:

021-08-21 14:08:57 ERROR (MainThread) [iotawattpy.connection] Err:
2021-08-21 14:08:57 ERROR (MainThread) [custom_components.iotawatt] Unexpected error fetching IoTaWatt data: 'NoneType' object has no attribute 'text'
Traceback (most recent call last):
File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 187, in _async_refresh
self.data = await self._async_update_data()
File "/config/custom_components/iotawatt/__init__.py", line 106, in _async_update_data
await self.api.update()
File "/usr/local/lib/python3.9/site-packages/iotawattpy/iotawatt.py", line 66, in update
await self._refreshSensors(timespan)
File "/usr/local/lib/python3.9/site-packages/iotawattpy/iotawatt.py", line 100, in _refreshSensors
results = response.text
AttributeError: 'NoneType' object has no attribute 'text'

I don't think this is related to the other issue at all but I thought it would be good to mention it (should I create a new issue for this?).

@gtdiehl
Copy link
Owner

gtdiehl commented Aug 30, 2021

@janecker The last issue you saw is the next issue I need to resolve. It's in the API library when retrieving the data. I need some retry. Though @jyavenard has made some changes I need to take a look at. Maybe it will resolve this one.

I'll take a look at the unique ID issue. It appears the mac address is not getting set, so the sensors don't have a unique ID.

@gtdiehl
Copy link
Owner

gtdiehl commented Aug 30, 2021

@janecker I found the problem in the API library. I'll try and fix it this week.

The mac address is only set during the connect() method. This method is only used during setup, so the mac address is not set when the setup goes to discover the sensors.

@gtdiehl
Copy link
Owner

gtdiehl commented Aug 30, 2021

@janecker I made the change and pushed an update. You can test it out. But what I found after I released is that new sensors will be created because of a new ID. The old ones will become unavailable and the new ones will be appended with _2. You have to remove the old unavailable sensors and rename the new ones.
Screen Shot 2021-08-30 at 3 07 22 PM

@jyavenard
Copy link
Contributor

Your last change only includes a bump in the version numbers.

It's unfortunate that it's not backward compatible with earlier versions, going to mess up history.

I wonder if there would be a way to allow for a smooth transition of system with a single iotawatt , so the first iotawatt doesn't need the Mac address, but secondary iota do.

@janecker
Copy link
Author

@gtdiehl: I tried the latest version and everything seems to work fine now, thank you!

@jyavenard: it is very unfurtnate that the naming changes and it took me 30-45 mins to rename / reassign everything but I am not sure if there really is a better solution for this. I think this was actually a bug if the mac address was not assigned as expected. At least all new users should get the proper assignment in future such that they to not have any issue when adding additional iotawatts in the future....

@jyavenard
Copy link
Contributor

And it’s going to change again :(

the official iotawatt integration used another unique_id format for the input sensor. And yet another for the output.

And that one too will change another time later once the iotawatt firmware is updated to provide a unique Id for the outputs.

@janecker
Copy link
Author

Oh i see, ok that gonna suck :(, is there any ETA on when the integration will be part of the core? Will this integration be integrated into the core or is there a parallel development going on?

@jyavenard
Copy link
Contributor

It's landed on dev ; will be in HA 2021.9

@jyavenard
Copy link
Contributor

I think the plan is to have the HACS version follow the official version with extra tweaks until it can be all merged back into HA core.

I've already rebased my changes on top of it there:
https://github.com/jyavenard/iotawatt_ha/tree/merge_HA

This adds an extra set of sensors, which is high accuracy energy readouts. Perfect to calculate import and export

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

No branches or pull requests

3 participants