mathieu-mp / homeassistant-intex-spa

Home Assistant integration for Intex Spa
https://github.com/mathieu-mp/homeassistant-intex-spa
MIT License
32 stars 6 forks source link

Use uid as device entity identifier #23

Closed mathieu-mp closed 2 years ago

mathieu-mp commented 2 years ago

Breaking change: devices have to be deleted then re-added

Elkropac commented 2 years ago

Hi, are we sure, that uid is really unique per device? I think we have not seen two uids from the same model yet. What if same model has same uid? Then you cannot have more spa's of same model in HA.

I used IP with uid as unique id, but HA documentation says not to use IP address in unique id

    unique_id = info['ip'] + ' ' + info['uid']
    unique_id = hashlib.md5(unique_id.encode())
    data['info'] = info
    data['info']['unique_id'] = unique_id.hexdigest()

https://github.com/Elkropac/hassio-intex-spa/blob/c7c89e2428b398bed54bc1cb2957edb021ee7010/custom_components/intex_spa/config_flow.py#L54

mathieu-mp commented 2 years ago

Hi, are we sure, that uid is really unique per device? I think we have not seen two uids from the same model yet. What if same model has same uid? Then you cannot have more spa's of same model in HA.

I agree, and I just assume uid means unique id. I'd like to find two people with the same spa model to validate this hypothesis.

I used IP with uid as unique id, but HA documentation says not to use IP address in unique id

Yeah, I read this recommandation from HA doc. That's why I wanted to avoid the current use of the spa name as the unique identifier. I am not so much into using another "forbidden" parameter to replace a previous "forbidden" parameter. I'll just stay on the above assumption for now, and fix it if eventually.

Elkropac commented 2 years ago

Hi, are we sure, that uid is really unique per device? I think we have not seen two uids from the same model yet. What if same model has same uid? Then you cannot have more spa's of same model in HA.

I agree, and I just assume uid means unique id. I'd like to find two people with the same spa model to validate this hypothesis.

I think it is ok for now. I guess there are not many people with more than one spa of the same model at home. Hope we can collect more data from people.

I used IP with uid as unique id, but HA documentation says not to use IP address in unique id

Yeah, I read this recommandation from HA doc. That's why I wanted to avoid the current use of the spa name as the unique identifier. I am not so much into using another "forbidden" parameter to replace a previous "forbidden" parameter. I'll just stay on the above assumption for now, and fix it if eventually.

Strictly speaking, I have not used IP adress as unique id in my code. It's just a hash of IP and UID together.

mathieu-mp commented 2 years ago

I think it is ok for now. (...) Hope we can collect more data from people.

Thank you very much for your contributions ! 🥇

mathieu-mp commented 2 years ago

I am not convinced with the use of info directly in the config entry, so I might move to populating the coordinator data with the info along with the status.

mathieu-mp commented 2 years ago

New pull request: #24

mathieu-mp commented 2 years ago

Implemented by #24