feature-requests icon indicating copy to clipboard operation
feature-requests copied to clipboard

Sprinkler - Allow multiplier to go down to 0.

Open Zebble opened this issue 3 years ago • 6 comments

An integration with HASmartIrrigation for weather-based would be great. If HASmartIrrigation determines that no watering should happen, it will provide a multiplier of 0. If this component could accept that value and not water if watering is requested through an automation, that would be handy.

Zebble avatar Sep 24 '22 20:09 Zebble

@kbx81

nagyrobi avatar Sep 25 '22 01:09 nagyrobi

Answered in https://github.com/esphome/feature-requests/issues/1844#issuecomment-1257061227

nagyrobi avatar Sep 25 '22 01:09 nagyrobi

I'm not strictly opposed to implementing/allowing this, but, I definitely have concerns that it introduces the potential for behavior that, while obvious as we discuss it, is not obvious to the end-user at the time the behavior presents itself. As I said, unless you happen to be staring at the logs at the right moment, you're not going to see why your sprinkler system won't turn on. Is there an easy way to address this situation that is 1. obvious to the user and 2. does not involve reading/monitoring logs?

kbx81 avatar Sep 25 '22 02:09 kbx81

I'm not strictly opposed to implementing/allowing this, but, I definitely have concerns that it introduces the potential for behavior that, while obvious as we discuss it, is not obvious to the end-user at the time the behavior presents itself. As I said, unless you happen to be staring at the logs at the right moment, you're not going to see why your sprinkler system won't turn on. Is there an easy way to address this situation that is 1. obvious to the user and 2. does not involve reading/monitoring logs?

Maybe an overall state field would be handy? Knowing what the sprinkler component is doing is something that is currently difficult. Some suggested statuses:

  • Idle
  • Paused (on valve #)
  • Cycle active
  • Queue active
  • Single valve active (for total x minutes)
  • Multiplier at 0/system deactivated

Zebble avatar Sep 25 '22 15:09 Zebble

That's an interesting point -- I guess I never thought interpreting its state was difficult, but then again I wrote it so I guess that kind of makes sense. 😆

You have a good suggestion with overall state -- let me think on it more...I have...some ideas...

kbx81 avatar Sep 25 '22 18:09 kbx81

Thanks @kbx81!

If it helps with context, I have been working on a standalone sprinkler system through this component. The ESP32 device can run completely on its own after it is initially configured through HA. HA isn't completely necessary as the defaults can also be set in the initial yaml. This also allows the system to keep running if it ever has connection problems to WiFi or HA.

So far this has worked really well! I had originally been using the Irrigation Unlimited add-on for HA and it's pretty good. It is completely reliant on HA though so if that connection ever goes down, nothing will happen on the irrigation controller.

I snagged a KC868-A8 from Kincony as soon as I saw this Sprinkler component appear. Everything has been running very well so far.

Thanks so much for all the hard work!

Zebble avatar Sep 25 '22 19:09 Zebble

@Zebble if you are still around -- I added a section to the doc outlining how to decipher the sprinkler controller's state. Would love some feedback. The preview is available here.

kbx81 avatar Dec 19 '22 07:12 kbx81

Hey @kbx81! Thanks for the update. Haven't had a chance to review, but will as soon as I can. Can you leave this open for another month or so?

Zebble avatar Jan 15 '23 18:01 Zebble

Hey @kbx81! Thanks for the update. Haven't had a chance to review, but will as soon as I can. Can you leave this open for another month or so?

No worries, it's not going anywhere. 😇

kbx81 avatar Jan 15 '23 18:01 kbx81

Thanks @kbx81.

Did take a quick look and tried using the total_cycle_time_enabled_valves() value. This doesn't appear to exist, so I assume your new documentation is for potential changes? So far after an initial review, it looks good. I like the updated total_cycle_time information, and the rest all seems to fit within the login I was hoping to implement.

Edit: Ah! I see the esphome #4159 update. Going to see if I can add that and test!

Zebble avatar Jan 15 '23 18:01 Zebble

Another update @kbx81,

I'm trying to test in a lambda:

      if (id(sprinkler_ctrlr).total_cycle_time_enabled_valves().has_value()) {
        // the total cycle time of enabled valves
        return id(sprinkler_ctrlr).total_cycle_time_enabled_valves().value();
      } else {
        return 0;
      }

I'm getting the following compile errors:

/config/esphome/irrigation-controller.yaml: In lambda function:
/config/esphome/irrigation-controller.yaml:812:62: error: request for member 'has_value' in 'sprinkler_ctrlr->esphome::sprinkler::Sprinkler::total_cycle_time_enabled_valves()', which is of non-class type 'uint32_t' {aka 'unsigned int'}
       if (id(sprinkler_ctrlr).total_cycle_time_enabled_valves().has_value()) {
                                                              ^~~~~~~~~
/config/esphome/irrigation-controller.yaml:814:67: error: request for member 'value' in 'sprinkler_ctrlr->esphome::sprinkler::Sprinkler::total_cycle_time_enabled_valves()', which is of non-class type 'uint32_t' {aka 'unsigned int'}
         return id(sprinkler_ctrlr).total_cycle_time_enabled_valves().value();
                                                                   ^~~~~

My C++ isn't particularly strong, so I'm trudging through it...

I included your updated component as such:

 external_components:
   # use sprinkler dev from kbx81
   - source: github://pr#4159
     components: [ sprinkler ]`

Zebble avatar Jan 15 '23 19:01 Zebble

total_cycle_time_enabled_valves() does not return an optional type. 😇

kbx81 avatar Jan 15 '23 19:01 kbx81

Ah! Yeah, that fixed it, thanks!

return id(sprinkler_ctrlr).total_cycle_time_enabled_valves();

I notice this returns a total including valve overlap and likely other settings, which is fantastic as I hadn't included that in my previous calculations. If I enable/disable valves, the value changes as expected. However, a quick question: When only a single valve is enabled, it includes the valve overlap in the total cycle time. Should valve overlap (and potentially other settings) apply when only a single valve is enabled?

This is probably an edge case so not particularly important.

Will check the rest of the bits as well. So far, looking good and should simplify my yaml!

Zebble avatar Jan 15 '23 19:01 Zebble

When only a single valve is enabled, it includes the valve overlap in the total cycle time. Should valve overlap (and potentially other settings) apply when only a single valve is enabled?

Ah ha, you found my off-by-one error. Classic programmer mistake. 😉 Will fix.

Thanks again for testing!

kbx81 avatar Jan 15 '23 20:01 kbx81

Thanks @kbx81!

Looking really good. The total_cycle_time_enabled_valves() and time_remaining_current_operation() are working really well. They will be valuable information on a controller interface.

I'm looking at the new divider option. I think having the divider/multiplier/repeat decoupled would be handy.

The multiplier would be good for using an overall control based on weather forecast and/or a plugin like HAsmartirrigation.

The divider could then be applied to sub-divide into number of soak times?

Repeat could be left alone as an overall repeater?

Maybe some calculations would help:

Total time = multiplier * (total zone times) * repeat

If a divider is applied, total time wouldn't change, but logic would divide the zone times by the divider, and repeat each cycle by the divider:

Total time = multipler * (zone times / divider) * repeat * divider

I'm sure there's a simpler way to describe this than what I'm providing, but I hope this makes sense?

Edit: I'm wondering if renaming "divider" to something like "soak times" would be helpful?

Zebble avatar Jan 15 '23 20:01 Zebble

The logic you describe makes sense, but what I was struggling with is the interaction between divider, multiplier and repeat. I get what you're saying, but I can't help but feel like the interaction between the three creates some pretty complex behavior that would be difficult to grok. I was going for "KISS", haha 😄

Edit: I completely understand what you are saying with respect to the operation of the multiplier, but...how would repeat work if the divider is set? Would we basically just be repeating repeats performed by the divider?

kbx81 avatar Jan 15 '23 20:01 kbx81

Good point... I'm going to let it percolate for a bit. I'm thinking of what other systems like rachio do. They don't have a repeat at all, just a divider/soaks. Multiplier always affects the total time.

Zebble avatar Jan 15 '23 20:01 Zebble

Sweet -- let me know! I don't have any strong feelings and I'm completely happy to re-work it a bit. I try to be cognizant of making things too complicated, but--in some regards, at least--we are probably way past that point anyway. 😆 In any event, please post back and let me/us know what you think! 😄

kbx81 avatar Jan 15 '23 20:01 kbx81

Hey @kbx81.

I did some digging on how Rachio does it. They have a really good writeup. Cycles and Soak

Surprisingly, I couldn't find anyone else that does something similar. The rest are very simple and manual. Most standalone controllers that have some sort of soak capability expect you to set the water time and soak time per zone and it's not very easy to adjust, or scale on the fly.

I like Rachio's method. It's smart and automates it based on the selected soil type. We could generalize this by adding a "max runtime" to each zone in the sprinkler declaration. This is how the basic logic could work:

  • Zone max is set to 8 minutes. This is governed by soil type (see the table about halfway down the page on the link above).
  • Zone is set to water for 15 minutes.
  • 15 minutes is more than 8 minutes so the logic divides the water interval as many times as it takes to get each interval under 8 minutes. In this case, it only needs to divide into 2 x 7.5 minute intervals.
  • Zone is watered for 7.5 minutes.
  • Rest of zones follow the same logic.
  • Comes back to water the first zone for its second interval - checks to make sure sure it's been at least 30 minutes (make this configurable per controller or zone?) since the end of the last interval.
  • If it hasn't, try the next zone interval.
  • If there are no zone intervals available after the 30 minute window but there are still zones in the queue, wait until there is an available zone (no watering done during this time).
  • Loop through this logic until all zones are complete.

I do realize this is far more complex than what is currently possible. I was thinking about how I could do this without any changes to the current component and sounds like queues could be used to pre-load the sequence and then run the queue. If that's the case, I wonder if this approach could be taken within the component itself?

This would also get rid of the need for a divider or repeat. A multiplier would still be valuable as this could adjust the overall schedule based on weather and evaporation / evapotranspiration from plugins like HAsmartirrigation.

Thoughts?

Zebble avatar Jan 16 '23 14:01 Zebble

Sorry for the slow reply -- I needed some time to think about all of this and then I got distracted with some other things. 😆

A couple of thoughts. First, I see how that is useful and, honestly, it's pretty amazing. That said, I'm a bit weary of building too much automation into the component itself. While there are most definitely common threads in terms of the way people use its functionality, one thing I've learned about this community is that we are all here because we want to do it our "own way". I'm more inclined to provide some good, solid basic tooling that allows folks an efficient way to tune the behavior to their specific use case and/or liking. I feel like if we build in a tool that just "does it all", we end up with a group of folks who spend their time trying to figure out ways to defeat that "smartness" and get it to behave the way they want because the "does it all" functionality just doesn't do it quite how they want. 😆

With that in mind, I like the "max runtime" concept -- I'm not sure I'll weasel that into this version/PR as I believe it'll require some re-working of the scheduling mechanism, especially as it relates to starting/stopping the system because (for example) only one zone remains. I need to let that ferment for a while and at the moment I've got a few other things I've been focused on. 😇

In any event, let me know what you think. Maybe I'm way off-base with those thoughts -- not sure. I do owe this PR another fix or two and I'll try to get that in within the next few days. 😇

kbx81 avatar Feb 01 '23 05:02 kbx81

Thanks @kbx81!

Sorry for the slow reply as well.

I agree 100%. It's impractical to cover everything. I'm happy to cover off the stuff I can do in Lambda code or within HA and keep the component doing the "basics".

If you're considering the "max runtime" concept then that would be great. The rest of the smarts that I outlined could be easily handled by Lambda/HA (time based on soil type for instance). Honestly, that was 100% my intent with what I suggested, and not for your component to do it all...

About the only other thing that would be great in the zone logic is a "minimum time between intervals" per zone (or controller?) to make sure the zone has time to absorb before the next interval.

To sum up, I think this is what I was thinking:

  • "max runtime" per zone
  • "minimum time between intervals" per zone
  • "multiplier" that can go down to 0 to control the total watering time

I can't think of an instance where "divide" and "repeat" would now be needed.

I'm willing to help. I'm just not sure where best to start!

Zebble avatar Feb 14 '23 03:02 Zebble

I can't think of an instance where "divide" and "repeat" would now be needed.

I probably wouldn't get rid of these because I do see use cases for them; repeat in particular is common/requested enough that people would complain if it was removed. 😇 I think that the divider action as it is right now is easy enough to grok that it probably also makes sense to keep in there.

As for min/max runtimes, I love the idea but I need to let that percolate for a while; it'll take some additional complexity built into the component's internal scheduler to handle that as it currently does not have a way to perform an autonomous stop/restart. I think an additional layer/wrapper will need to be created to permit this to happen which will probably make more sense to build into a subsequent PR (this one has already become pretty unwieldy, haha).

As for your original "multiplier down to zero" request, that's implemented along with a few other things so you'll have that for sure! 🍻🎉

kbx81 avatar Feb 14 '23 04:02 kbx81