Fix pause within pause

[curlydingo]

This fixes the issue raised in vinteo/hass-opensprinkler#276.

It passes in my environment, but given the nature of it I think it makes sense to be well tested by someone else before we merge/release it.

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[EdLeckert]

I couldn't get it to work without adding a delay:

        if seconds > 0 and self.pause_active:
            await self._set_pause(0)
            time.sleep(1)

[curlydingo]

Hmm I was afraid of this. I tried to avoid such a delay because I think the sprinklers will turn on (for that 1 second) before pausing again. You might get away with it dependingon the kind of solenoids you have but it doesn't feel like a good solution.

Out of curiosity, what hardware are you running OpenSprinkler on @EdLeckert? I have Rpi3+ & OpenSprinklerPi and didn't need a delay, but it seems I was right to worry about the timing issues on other platforms.

If this is all true we might genuinely need to fix it in the sprinkler firmware instead, which will take a bit longer.

Thanks for trying it out anyway.

[EdLeckert]

Out of curiosity, what hardware are you running OpenSprinkler on @EdLeckert?

OS3.0 AC-powered with wired Ethernet module

Hmm I was afraid of this. I tried to avoid such a delay because I think the sprinklers will turn on (for that 1 second) before pausing again.

Probably. My sprinklers are blown out for the winter, but I'm sure they would. I never said it was a good idea. 😏

If this is all true we might genuinely need to fix it in the sprinkler firmware instead, which will take a bit longer.

This is absolutely an API problem, either with the firmware or the docs. Have we tried reporting it? We should.

But even if the firmware were changed tomorrow, some users would never get the fix. So I think we should again take a hint from the UI and assume that the docs are wrong. Once paused, the only thing you can do from the UI is unpause, and this is the worldview that most users will have anyway, not the API docs.

Unless you have a better idea, I suggest we throw an error if already paused, and document this behavior. Users could use something like a conditional card to provide an appropriate UI experience similar to the OpenSprinkler UI.

[vinteo]

Unless you have a better idea, I suggest we throw an error if already paused, and document this behavior. Users could use something like a conditional card to provide an appropriate UI experience similar to the OpenSprinkler UI.

I agree with this. My opinion is pyopensprinkler should definitely throw an error if already paused. But hass-opensprinkler can handle this and check if pause active when calling a service and then doing nothing or throw an error, I am not that familiar with how HA handles errors in services. Then for automations and HA UI, its up to the user to handle the scenario by unpausing first if they want to

[EdLeckert]

This all seems a bit awkward, so I've been thinking about how the UI could work. As an alternative to a Conditional Card, a Template Switch might also work. A Template Switch combined with an Input Number or Input Datetime would be "on" if the controller is paused, and the user would have to turn it "off" and back "on" for a new pause duration to take affect. We could document an example of how this would look.

[curlydingo]

Haven't had any time to work on this, but have just raised OpenSprinkler/OpenSprinkler-Firmware#258 to see what they think.

I largely agree with @EdLeckert's latest suggestion but think it should be a switch & number we provide in the integration rather than a template the user has to set up themselves, just to make it easier.