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

Ask for PIN in Husqvarna Autmower BLE integration #135440

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

alistair23
Copy link
Contributor

Breaking change

The integration now requires the AutoMower PIN when being setup. This ensure Home Assistant can communicate with more models of Mowers and with higher security levels.

Proposed change

All Automowers are setup with a 4 digit PIN. Depending the the model of the Automower and the security level set on the device by the user the PIN is required at boot and when performing certain operations.

The current Home Assistant integration doesn't require a PIN. It seems like higher security levels or certain models always require a PIN though. So the integration currently doesn't work for all models or configurations.

As such, let's request a PIN from users when setting up the integration so that we can use that for communicating with the mower. This should make the integration more robust for a range of different models and security levels as we can send the PIN as part of the BLE setup.

Fixes: #131321

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)
  • 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

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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

  • The manifest file 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.
  • 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:

All Automowers are setup with a 4 digit PIN. Depending the the model of
the Automower and the security level set on the device by the user the
PIN is required at boot and when performing certain operations.

The current Home Assistant integration doesn't require a PIN for certain
models. It seems like higher security levels or certain models always
require a PIN though. So the integration currently doesn't work for all
models or configurations.

As such, let's request a PIN from users when setting up the integration
so that we can use that for communicating with the mower. This should
make the integration more robust for a range of different models and
security levels as we can send the PIN as part of the BLE setup.

Fixes: home-assistant#131321
Signed-off-by: Alistair Francis <alistair@alistair23.me>
@alistair23 alistair23 force-pushed the alistair/husqvarna-pin branch from d7a234e to 86f32fd Compare January 12, 2025 11:56
Copy link
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why do we need this now and what happens to the old config entry data for people who already have this set up?

Comment on lines +73 to +74
await self.async_set_unique_id(self.address, raise_on_progress=False)
self._abort_if_unique_id_configured()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have the unique id set

return await self.async_step_confirm()
return await self.async_step_bluetooth_confirm()

async def async_step_bluetooth_confirm(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, Do we now have

Discovery -> bluetooth_confirm form -> confirm form -> create_entry?
User form -> confirm form -> create_entry?

Let's try to remove that confirm now, since clicking submit on the page to fill in the PIN sounds like a good enough confirm to me. Same for the user flow.

@home-assistant home-assistant bot marked this pull request as draft January 13, 2025 11:48
@home-assistant
Copy link

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

Learn more about our pull request process.

@joostlek
Copy link
Member

Oh I read the description now (bad habit lol).

So I think instead of marking this a breaking change and have people setup their device again, let's create a way to update their configuration. There are 2 ways to do this and I need some more information about the PIN to know which one makes more sense:

  1. Reauth flow, where we ask the user to reauthenticate with the device, so the user can put in the PIN. Benefit of this is that we can have everyone set the PIN or the integration does not work (as in, that is not a benefit, but it does what it should without adding a lot of if CONF_PIN in entry.data for when you add more features in the future that require that PIN. But my question here would be, can the PIN change?
  2. A repair flow, where we kindly ask the user to provide a PIN and then just keep working like before. More graceful, but harder to maintain.

@joostlek joostlek changed the title components/husqvarna_automower_ble: Support PIN Ask for PIN in Husqvarna Autmower BLE integration Jan 13, 2025
@joostlek
Copy link
Member

Someone just told me that the PIN can be changed, so I would recommend using a reauth flow to allow the user to provide the new PIN

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cant connect to Gardena Sileno City 250
2 participants