Add BleBox integration

[gadgetmobile]

Proposed change

This is a WIP for a complete/quality-oriented integration for BleBox smart home devices.

Features:

• 11 BleBox products are implemented. (PR reduced to 1 platform/3 devices)
• almost 100% coverage through tests (which mock out 100% of the client PyPI lib)
• minimal, bare-bones implemenation on Hass side (code is almost 100% Hass-specific)
• separate PyPI client library doing all the work (with its end-to-end functional tests)

NOTE: I first wanted some feedback for the overall approach and testing
strategy - so pick a single platform yet (as a first PR) didn't seem to make
sense. (Let me know if I'm wrong and I can reduce this PR to one platform).

I'd also really appreciate some tips and feedback on:

• overall do's/don'ts in Hass integrations (that I got wrong or missed)
• what other major changes are needed to make this a decent PR?
• how I can make reviewing easier

Also, thank you all for putting so much care and time into making Home
Assistant such a rich and high quality automation platform!

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request: add docs for upcoming BleBox integration home-assistant.io#13148

Issues / questions

How should I properly handle ...

• ... async_update - when multiple entities use same device (throttling? via @Throttle?)
• ... configuration (via config_flow and otherwise) - best practices ?
• ... device identification - async ? (instead of synchronously during setup)
• ... persistently storing state (e.g. "last on color/brightness")?
• ... misc sensors - "as many as possible" or conservatively (only when requested by users)?
• ... exceptions - are harmless call stacks ok in logs? or should I always catch+log errors?
• ... serializing requests to devices (due to device limitations)

(Most of the remaining things I was unsure about are marked as TODO's in the code).

Any more tips/guidance to make this a great integration are appreciated.

Checklist

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

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

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

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

• The [manifest file][manifest-docs] has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• Untested files have been added to .coveragerc.

The integration reached or maintains the following [Integration Quality Scale][quality-scale]:

• No score or internal
• 🥈 Silver
• 🥇 Gold
• 🏆 Platinum

[homeassistant]

Hi @gadgetmobile,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[gadgetmobile]

NOTE: Build failures are only due to pylint returning '4' (due to harmless TODO's):

https://dev.azure.com/home-assistant/Core/_build/results?buildId=31768&view=logs&j=8b66295a-05e1-52cf-5640-7a4f4e1975e2&t=6c4465fe-285a-5650-787c-2e3dbe1da192&l=48

(Most TODO's mark missing coverage points, the remainder are improvement suggestions).

[frenck]

Please reduce the PR to a single platform. These kinds of large PR's are hard to review. As per: https://developers.home-assistant.io/docs/creating_component_code_review#5-make-your-pull-request-as-small-as-possible

As for the TODO's; In general, it is better to make it trackable, having it in code often ends up being there forever. The rules are enabled for that reason. If you really have the intention to fix those; create a tracking issue after the PR is merged.

... async_update - when multiple entities use same device (throttling? via @Throttle?)

https://developers.home-assistant.io/docs/integration_fetching_data/#request-parallelism

... configuration (via config_flow and otherwise) - best practices ?

Not sure about your question here?

... device identification - async ? (instead of synchronously during setup)

Not sure about your question here. If a connection is needed for device identification, that is possible (Hue & WLED do this as well). It should not block our event loop in those cases.

... persistently storing state (e.g. "last on color/brightness")?

Those are restored automatically for Lights.

... misc sensors - "as many as possible" or conservatively (only when requested by users)?

In general, we try to make the most usable data available in separate entities. However, we do recommend to disable less common ones by default. This way a user can enable them if they want.

https://developers.home-assistant.io/docs/entity_index#advanced-properties

... exceptions - are harmless call stacks ok in logs? or should I always catch+log errors?

Logs should be usable and readable for the average user, stack traces are not part of that. Logging more extensively in the debug level is advised.

... serializing requests to devices (due to device limitations)

What is your question here?

[gadgetmobile]

Please reduce the PR to a single platform.

Done. (Only covers + config flow).

As for the TODO's (...)

I planned to deal with all of them before squashing the commits. I've removed all of them. (Along with pylint comments disabling them). Coverage is also now at 100%. (Though I wish there were standardized ways to test things like async_setup, etc.).

https://developers.home-assistant.io/docs/integration_fetching_data/#request-parallelism

Thanks! I must have missed it. In my case, I understand it's "0" (since every entity defines async_update) and it's sufficient for me to use a semaphore in the client library.

... configuration (via config_flow and otherwise) - best practices ?

Not sure about your question here?

All I really need to configure is the IP address (and port, though it's always 80). I'm not sure what other cases to handle, if any. (Like preventing adding and already existing device/ip - which I'm doing already).

If a connection is needed for device identification, that is possible (Hue & WLED do this as well).

I didn't know WLED was a good example, but I seem to be doing exactly the same thing - except for handling the mac address (with async_set_unique_id and _abort_if_unique_id_configured). I'll change that later.

In general, we try to make the most usable data available in separate entities. However, we do recommend to disable less common ones by default.

Makes sense, thanks. I'll keep that in mind once more sensors are requested.

Logs should be usable and readable for the average user, stack traces are not part of that.

Thanks. I'll make sure handle them appropriately (along with proper tests and coverage).

Follow up questions:

1. BleBox devices are feature-rich wireless devices that speak HTTP/JSON. Are there any recommendations or other integrations worth looking at or comparing to (beside HUE and WLED)?
2. Should I add the physical devices to the registry? (Is it useful or clutter?)
3. BleBox devices generally respond to commands (POST) with complete current state - should I do something to skip unnecessary/immediate async_update calls when this happens? (I was thinking of just skipping/throttling updates in the client library based on recency of data).
4. I am confused as to when async_setup_platform is required to be implemented. Do I need it?

Thanks again for helping me getting this done right! (I really appreciate it!)

[bdraco]

Hi @gadgetmobile Thanks for submitting this integration! Please take a look at my review comments.

[bdraco]

Hi @gadgetmobile Thanks for working through these. There are a few more things to address.

[gadgetmobile]

Changes since last review:

• Rebased to dev branch
• 548930e using ConfigEntryNotReady properly, setup improved (Api client setup moved to init).
• 7a48577 blebox uniapi bump to 1.3.1

I'll double check to make sure I haven't missed anything.

Let me know if there's anything else I can correct/improve, thanks!

[bdraco]

@gadgetmobile Seems like this is getting close to merge, would you please recheck the docs to make sure they are still accurate.

[gadgetmobile]

@gadgetmobile Seems like this is getting close to merge, would you please recheck the docs to make sure they are still accurate.

Once the review is finished here, I'll be sure have the docs updated (and the PR linked here) before I squash all the above commits.

Thanks for all your help and support so far!

[bdraco]

@gadgetmobile No need to squash, we do that on merge

[gadgetmobile]

I addressed all issues (in order) in commits 2d97e5e..bd70c23 and rebased.

I'm finally beginning to like how the cover tests look - thanks for the help making them so much better!

[gadgetmobile]

I think I've covered (no pun intended) all the suggestions - and I don't have any further ideas what to improve.

Feel free to be as picky as you like, so that I'll know how to properly prepare the next PR (after this one).

[MartinHjelmare]

Looks good! Some comments.

[MartinHjelmare]

Great!

[bdraco]

Nice cleanups since the last time I looked this over. @MartinHjelmare Thanks for jumping in.

[gadgetmobile]

Big thanks to both of you for helping me out so much here!

[bdraco]

Looks like one test left to fix
FAILED tests/components/blebox/test_cover.py::test_stop[pyloop-gatebox] - Ass...

[gadgetmobile]

That test started failing for me locally only after a rebase (likely related to #35073 changes).

To fix it, I removed that test case (since "cover stop service" wouldn't be available, because stop wouldn't be supported - so it was testing an impossible case).

I pushed this without the rebase for clarity. (And all tests now should pass with and without a rebase anyway).

I'm expecting CI to succeed now.