sebr / bhyve-home-assistant

Orbit BHyve custom component for Home Assistant
MIT License
252 stars 42 forks source link

Smart watering program fix #101

Closed allistermaguire closed 1 year ago

allistermaguire commented 2 years ago

This PR fixes an issue with B-hyve Smart Irrigation Controllers that have multiple zones/stations. The Smart Watering program id is not constant when the program updates. This causes the following issues:

  1. 'Irrigation Smart Watering program' switches are create each time there is a change.
  2. Error when trying to turn off the above switch due to the program_id in the module and the id on the B-hyve being out of sync.

I have resolved this by creating a constant program id for programs that are Smart watering based off the device id. This shouldn't be an issue based on my assumption that you can only have one Smart watering program per device.

sebr commented 2 years ago

@allistermaguire Thanks so much for the info and the PR.

Can you share more details about the specific discrepancies in the API which cause this to happen? Any API responses or websocket payloads that demonstrate this behaviour would be valuable to validate the logic.

This is particularly important as changing the unique_id is a breaking change (home assistant will create new entities and the user will be left with orphaned entities and need to do manual migrations)

Thank you!

allistermaguire commented 2 years ago

Hi @sebr, that is a good point. I did not consider that as it is already broken for me, I already had multiple orphaned entities which I needed to clean up. But considering no one else has reported it is likely most people aren't using Orbit Hyve controllers with multiple zones and therefore not having this issue.

See smart_watering_program_messages_json.txt for program_changed json. I have formatted it nicely so it is easy to read include removing "watering_plan" and "long_term_program" as they are relevant and make the json really large.

allistermaguire commented 2 years ago

I am going to have to rebase this one. I noticed that it includes my naming changes for zones and sensors.

I have submitted PR #102 which is possibly more valuable to users, then can circle back to this one later.

allistermaguire commented 2 years ago

I have rebased and tidy up the code so it is more readable. I have also limited the change to Smart Watering programs, but it is still breaking.

The only way I could find to migrate unique_id is if the integration uses config entires.

julian-yang commented 2 years ago

Hi, just curious what are the next steps to get this PR merged in?

I think I am in a similar boat as allistermaguire. I'm using Smart watering on multiple zones, and the next watering time only seems to show for certain zones, and the zones that it shows for changes. Seems like it may be due to the "Smart watering program not having a constant ID" issue mentioned here.

allistermaguire commented 2 years ago

I have updated this PR so that it shouldn't be a breaking change. Now that it is using config entries I can migrate the Unique ID for the Smart Watering program switch.

It uses device_id for both the unique_id and constant program_id. This works fine for my use case as I have a single device with multiple zones. I am not sure what the effect is if you have multiple devices.

gdgeist commented 1 year ago

This issue still persists - whats up with this pull request?

allistermaguire commented 1 year ago

I haven't checked the latest version/s, but I haven't seen anyone else raise this issue. Not sure if most people mainly run the smart hose timers vs the irrigation controllers as I do. I am personally running my forked version that I update, but haven't done so since Feb 23.