Add SensorPush Cloud integration

[sstallion]

Proposed change

This PR adds cloud integration for SensorPush devices. It communicates with the publicly available Cloud API using the sensorpush-api Python package. Care was taken to ensure that presented devices appeared the same as those created by the existing SensorPush integration. A G1 WiFi Gateway is required to make use of the Cloud API.

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [x] New integration (you're welcome!)
• [ ] New feature (which adds functionality to an existing integration)
• [ ] Deprecation (breaking change to happen in the future)
• [ ] Breaking change (fix/feature causing existing functionality to break)
• [ ] Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/33735

Checklist

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] I have followed the perfect PR recommendations
• [x] The code has been formatted using Ruff (ruff format homeassistant tests)
• [x] Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• [x] Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• [x] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [x] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [x] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• [x] I have reviewed two other open pull requests in this repository.

1. https://github.com/home-assistant/core/pull/121863#pullrequestreview-2176476754
2. https://github.com/home-assistant/core/pull/121805#pullrequestreview-2176477135

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[sstallion]

It's been a while since I last left a review, so please take all of this with a grain of salt. I may be incorrect, so don't take it all as absolute truth.

No worries, this is my first integration - I appreciate another set of eyes!

[sstallion]

Ping! (Keeping PR from going stale)

[sstallion]

Thanks for the review @joostlek! I wanted to check if there was anything preventing this PR from merging. The integration has been working well for some time now and I wanted to double check before I made major changes to the config flow. I also wasn't sure if you wanted me to create a custom API library versus using the existing Swagger client or if that was personal preference. Thanks again!

[joostlek]

We probably still have a few rounds to go. But I kinda dislike all the things in the integration that are needed to complete the library. We'd rather have a more complete library. But it's not a blocker as this isn't the only one, but I still wanted to express my opinions on that :)

[sstallion]

We probably still have a few rounds to go. But I kinda dislike all the things in the integration that are needed to complete the library. We'd rather have a more complete library. But it's not a blocker as this isn't the only one, but I still wanted to express my opinions on that :)

I understand - I'm not the biggest fan of this approach either. I don't have a good feeling for when/if SensorPush will make changes to the API (the most recent update I can find was from a few weeks ago). Once the API settles I can put together something that would be much nicer to integrate.

With respect to the remaining issues to address, what do you see as blockers to merge the PR?

[joostlek]

I did ask around for you. Is it possible to create the openapi as an inner folder in the dependency and just have a public facing facade with nice and neat functions?

/Mylib
         | _api/ (autogen)
         | __init__.py
         | client.py (wrap api)

[sstallion]

I did ask around for you. Is it possible to create the openapi as an inner folder in the dependency and just have a public facing facade with nice and neat functions?

Thanks! Not really - Swagger generates an entire setuptools-based package for Python clients. I've also released this to PyPI before opening this PR, which might make it a little tough to back out.

The only other alternative I can think of is to create a new package which depends on the Swagger client and presents the same interface as api.py. The only consumer of this new package would be Home Assistant, which doesn't seem quite right since it would create a cyclic dependency (eg. this library would have to rely on both hass and sensorpush_api to do its job unless we want to use the generic Python executor)

[joostlek]

I think there are more ways of generating them, I've also seen them async

[joostlek]

Cc @mj23000, you know more about this I think

[edenhaus]

What is the difference between the local and cloud sensor push integration regarding functionality? What features will be offered only by the local one and which ones only by the cloud one?

[mj23000]

Are you using swagger-codegen for the API generation? We use openapi-generator , which seems to be the more up to date OpenAPI generator. An async Python client can easily be generated by using the python-pydantic-v1(As Home Assistant still uses pydantic v1 for now) generator and the asyncio backend. Getting everything to work as expected can be a bit of a pain, which results in our generation being pretty complicated as we modify many of the generated files.

[joostlek]

But I personally do not allow any more Pydantic V1 integrationd without shims for v2

[mj23000]

But I personally do not allow any more Pydantic V1 integrationd without shims for v2

Yeah exactly, which is part of what makes our generation complicated. It looks like the API client isn't too big to manually add the shims, so you can probably avoid too much work.

[joostlek]

Thanks for giving the extra info @mj23000, much appreciated!

[sstallion]

What is the difference between the local and cloud sensor push integration regarding functionality? What features will be offered only by the local one and which ones only by the cloud one?

There should be no difference between the two; even the polling intervals are the same. At the moment, it does not appear that the BLE version of the integration provides every possible sensor but that's likely just a limitation in the sensorpush-ble library.

[sstallion]

Are you using swagger-codegen for the API generation? We use openapi-generator , which seems to be the more up to date OpenAPI generator. An async Python client can easily be generated by using the python-pydantic-v1(As Home Assistant still uses pydantic v1 for now) generator and the asyncio backend. Getting everything to work as expected can be a bit of a pain, which results in our generation being pretty complicated as we modify many of the generated files.

Gotcha. Is there an exemplar I could use to pattern this integration after?

[sstallion]

@joostlek It sounds like the generated sensorpush-api library is a no-go for integration. Since it sounds like there are better generation tools available that library will need an overhaul. With respect to adding this new library, should this be added as an internal package since it will need modification or does this still need to be an external library published to PyPI?

[joostlek]

I mean, I think you got the idea now that it should not be just generate and publish and call it a day.

We still require all device specific logic to be its own library, so that's a must.

[mj23000]

Are you using swagger-codegen for the API generation? We use openapi-generator , which seems to be the more up to date OpenAPI generator. An async Python client can easily be generated by using the python-pydantic-v1(As Home Assistant still uses pydantic v1 for now) generator and the asyncio backend. Getting everything to work as expected can be a bit of a pain, which results in our generation being pretty complicated as we modify many of the generated files.

Gotcha. Is there an exemplar I could use to pattern this integration after?

Yes, the mozart-open-api Python client can be used as an example (And is used in the bang_olufsen integration). This client is modified in various ways, so the output of the OpenAPI-generator will be a bit different, but not majorly.

To make Pydantic v1/v2 work, ensure that the Pydantic v1 imports are wrapped in a try catch like this:

try:
    from pydantic.v1 import Field, StrictInt, StrictStr
except ImportError:
    from pydantic import Field, StrictInt, StrictStr

This may unfortunately break some typing checks.

[sstallion]

Heads up, I'm still working on this though I'm somewhat slowed by travel for work. I should have something ready for review late next week.

[sstallion]

Finally back from travel and getting some eyes on this. I'm hoping to have this closed out by the end of the week.

[sstallion]

@joostlek @mj23000 I've managed to update sensorpush-api package using openapi-generator with a number of modifications to support Pydantic v1 and v2. I've removed the old async shims for the old API and I'm back to updating the integration with requested changes.

Once thing I'm having a tough time with is the request to move the contents of api.py into the sensorpush-api package. Now that the old shim are gone, the remaining functionality is HA-specific, I'm not sure this makes much sense to move into sensorpush-api. I've looked through other components in the tree and it looks like many have helper modules similar to api.py - is this a deal-breaker?

[ZyberSE]

I just wanted to shim in with some observations I've made during beta testing this integration.

1. Sensors sometime (rather frequent) become unavailable for a moment.
2. When renaming a sensor in the SensorPush app it's not reflected in Home Assistant but the sensor becomes unavailable.
3. When adding a new sensor in the SensorPush app it does not show up in Home Assistant.
4. All sensors have pressure and atmospheric_pressure properties even though they do not support it (only HTP.xw does).
5. Battery property is named Voltage which I argue should be Battery as that reflects what it is, its value correctly have V as unit though. Example: Voltage: 2.54 V, I argue Battery 2.54 V is better.

Beside above observations the integration has been working well for me in core 2024.8.2. I've also briefly tested it in the Docker dev environment where it also worked. My setup consists of 18 SensorPush sensors and 1 gateway. I've previously run the (old) SensorPush integration avaliable in HACS and also the current, official SensorPush (Bluetooth) integration.

There was a question higher up in the thread about why this is needed when we already have the Bluetooth integration -- The BT integration works ok but is limited to a subset of data due to the data struct isn't public information and only parts of it have been reverse engineered. I've been in long discussions with SensorPush about sharing the spec but they for some reason do not want todo that and referee to the SensorPush Bluetooth API, which unfortunately is limited to the new sensors and excludes the original HT1 sensor. This integration on the other hand supports all sensors and all properties, so it's definitely a welcomed addition.

There is room for future improvement if Home Assistant would allow for it.

1. The current implementation grabs only the latest sample from sensors but the API supports fetching historic data. If that could be implemented we could better handle network outages, reboots etc. by simply asking for all new values since last received sample for each sensor.
2. It would also open up to the possibility to import historic data on first run.

[sstallion]

@joostlek @edenhaus Thanks for bearing with my travel schedule - I've made the requested changes. The integration should be much cleaner. Everything is now natively async and sensorpush-api has been updated to support Pydantic v1 and v2 with shims. api.py is now much smaller and no longer contains workarounds to the updated library.I haven't squashed the branch yet to make review a little easier. Once approved, I can squash and rebase. Thanks in advance!

[sstallion]

@joostlek I've moved as much as I can into a new package named sensorpush-ha. Please take a look and let me know if this is acceptable. With respect to the requested changes in coordinator.py, I wasn't able to come up with a clean way to support this without creating a circular dependency. It's a very small amount of glue to tie the two API objects together and I would like to keep this as is if possible.

[sstallion]

@joostlek I've had some time to think about adding another package to hide the necessary integration code for SensorPush and I do not think this is a reasonable approach. This PR has been open for a month and a half with almost a hundred comments. I have put forth a best faith effort to support both SensorPush's wishes for API integration using Swagger and Home Assistant's development requirements and I feel we're at an impasse.

I have put a significant effort into following documented guidelines for contributing to core and have done my best to apply all requested changes. I can't in good conscience add another package to PyPI that contains around 100 lines for integration code that is only used by this HA integration. What is presented has been tested and used for months at this point and I have to move on to other projects.

I very much want this to be a part of core so that other people can make use of this, but we have spent more time on this PR for a small integration than has gone into much larger features. I am not sure I understand this level of scrutiny, especially given that I am following patterns that already exist in core and are in common use today. Changes that have been requested have largely been subjective with no developer documentation to provide context.

If these changes are still not considered acceptable, please let me know. I will close this PR and use a custom integration. This is my first contribution to Home Assistant and I hope there will be more in the future.

CC: @balloob @frenck

[ZyberSE]

I've been using the first iteration of this for a month now, works great! Looking forward to see it merged into core.

[sstallion]

@joostlek Checking in - do you have any thoughts?

[sstallion]

@joostlek Checking in again. I understand this isn't ideal, but I'm hoping you can meet me halfway.