kvj / hass_nuki_ng

Better support for Nuki devices in the Home Assistant
MIT License
177 stars 34 forks source link

Integration is local_push but manifest says local_polling #10

Open kongo09 opened 2 years ago

kongo09 commented 2 years ago

As far as I understand, the integration registers a callback URL in the bridge and processes the callbacks. Shouldn't that classify the integration as local_push instead of local_polling as it is stated in the manifest right now?

alexdelprete commented 2 years ago

It does both. I don't know if they can both be specified in the manifest.

kongo09 commented 2 years ago

If it does both, I'm inclined to advocate for local_push as this reflects the real-time nature of the integration. I would expect that most integrations that are local_push can also poll if they see the need.

alexdelprete commented 2 years ago

I would put them both, separated by a comma. Maybe it works but it's not documented. Worth a try...

kongo09 commented 2 years ago

After reading https://www.home-assistant.io/blog/2016/02/12/classifying-the-internet-of-things/#classifiers it is clear to me that an integration that is notified by the device about a state change is classified as local_push. Ideally, these devices (and integrations) still allow polling, so that state is available, e.g., after Home Assistant startup.

alexdelprete commented 2 years ago

I already read that document many times, and the reverse could also be stated: the two categories are not mutually exclusive, and one does not contain the other. nuki_ng offers direct communication with the device through push AND polling.

But really, I don't think it's so important to require a long discussion, it's up to @kvj to decide.

The real issue is that you can you choose only one, but I don't know if that's even true, if it is, HA devs should change that and allow multiple choices.

kvj commented 2 years ago

Strictly speaking, as the component supports Web API endpoints, it should be cloud_polling. But, as this value is not dynamic and doesn't change anything, it doesn't matter which value to use

alexdelprete commented 2 years ago

Correct. It should be possible to have a list vs choosing only one. And I agree it doesn't really matter. It changes a small icon on the upper right of the integration box in the integration list...

kongo09 commented 2 years ago

All fair, maybe we should just close this. I guess I was coming a bit from a marketing perspective, especially in comparison to the default Nuki integration which cannot do push at all and the HA documentation which calls push the best of the best.