Water Backup Logic Development

[TerryJC]

After the discussion at viewtopic.php?p=5812#p5812 I have made a tentative start on this work (it took me quite some time to work out how to do the 'between' test in the if statement. )

Anyway, I've got that far and I'm ready to actually create the new function. I've created a new branch in my local git repository (water-backup-dev). Within that branch I've created a modified version of controllogic.py with the conditional code at the top of the sumppi_control_logic() function, as suggested by Hamish. During Opening Hours this is ignored but between 1600 UTC and 0900 UTC, the new function water_backup_logic() will be called. However, I have some rooky questions:

1. I''m assuming that the new function should be created within controllogic.py. Is that the right approach or should there be a different file.?
2. I suspect that once the the new function is executing there will be some time before the levels stabilise. This leads to the possibility that the function won't return before a new tick is due. Also the new function will need access to the readings so that it can know when to exit. How much of the sumppi_control_logic() code should be repeated within the new function?
3. Alternatively, I could set a timer so that the new function is exited periodically to update the readings and ticks. Or as another alternative, I could move the conditional code until after the readings have been done, just before the logic starts when the levels are checked. Thoughts?
4. When I have some code that I am happy with, I would appreciate it being Peer Reviewed. (Hamish?)
5. I don't think it's going to be easy for me to test this code before I deploy it. Hamish will you have time to load it into your simulator?
6. What is the best way to deploy the finished code?

[hamishmb]

Okay.

1) Yes, I would put the function in there, probably next to sumppi_control_logic() seeing as it runs on Sump Pi.

2) There are some legacy arguments that aren't used in sumppi_control_logic() that you'll need to copy, but I will eventually go through and clean that up everywhere. Your function shouldn't need to keep running longer than just to set the pump and valve states, and then it will be called (via sumppi_control_logic()) every reading interval so it can decide if it needs to change things, just like sumppi_control_logic(). You don't need to wait for the levels to change or stabilise, so this should be a non-issue. You should just be able to copy what sumppi_control_logic() does to access the readings.

3) Probably unnecessary, see my answer to 2).

4) I'm happy to peer review it. I need to update the River System VMs first, but that's fine as I need to get on with that anyway.

5) Yep, I'll test it for you when I peer review it.

6) I have a site-wide updater, but it is currently broken. I'll merge your code into the master branch when we're happy with it and then I'm happy to deploy it for you, at least until I've sorted the site-wider updater, at which point that can be used to deploy all new versions of the code.

[TerryJC]

Hamish,

Thanks for your quick response. I've got to take someone to a Hospital appointment shortly, so I won't get back onto it until tomorrow.

I'll add any more queries that I identify here (hopefully none).

[TerryJC]

Hamish,

One other question. As far as I can tell the job of the new function is to empty the sump in the shortest possible time, so I am only reading in two parameters; readings and devices. Unless I've missed something I shouldn't need monitors or sockets because all that is going on in the background, but what about reading_interval? Is there any benefit in me setting the reading interval to something low? If so, I'll return that value at the first iteration or I could do that before the function is called in the if structure. However, the main object of having a low reading interval was to ensure that the main circulation pump was turned off before the water ran out, so it's not needed for that. However, the butts pump might end up with no water at its inlet, so we might need a low value for that. At the next iteration of the function after the level falls below 300 mm the butts pump will be disabled anyway, but I'm not sure if that might be too long if the reading interval was long when the function is first called.

If I don't need reading interval, then I believe that there doesn't need to be a return value.

What do you think?

[TerryJC]

I had a telephone discussion with Penri this morning regarding the risk of allowing the Butts Pump to continue for one reading cycle after the sump level reached 300 mm. The background to my question was to establish if i needed to increase the reading rate as the Sump emptied and the consensus was no.

However, what did emerge during the discussion is that my original strategy of immediately turning off the main circulation pump was not a good idea, because there is insufficient backup storage for the full amount of water that could drain from the river / bog garden once the pump was off.

The new strategy will therefore be:

1. Close the Gate Valve.
2. If the Sump Level is greater than 300 mm and the Butts have capacity; turn on the butts pump.
3. If the Sump level is less than 300 mm turn off the main circulation pump and the butts pump.
4. Keep checking levels on each iteration of the function, turning the pumps on and off as above until the butts are full and the sump is empty.

Clearly, until the Lady Hanham and Stage butts are connected, water could still be lost down the overflow, but not as much as if the main circulation pump was off all the time. Even after the extra capacity is available there could be some loss, but that is more likely to occur when there has been a lot of rain.

[hamishmb]

Hi Terry,

The new strategy sounds good, just make sure you explicitly turn the circulation pump on at 300mm too (assuming that was the intention).

I would always say that if you have multiple levels to trigger different things, always set the pump states - don't make the assumption that all the levels are triggered at the probe floats up and down, as this isnt always the case.

Those arguments are legacy ones that I need to remove and are no longer used IIRC - you shouldn't need them. Make sure the return the reading interval after the work in your new method is done - returning anything immediately exits the method.

You could do something like this in sumppi_control_logic():

Code: Select all

return night_logic()

So that sumppi_control_logic() will return whatever interval you like and you can control it from your new function but returning a value from within that.

I'm having a slightly bad autism day so if that's not very clear I shall clarify it later

[TerryJC]

You could do something like this in sumppi_control_logic():

Code: Select all

return night_logic()

So that sumppi_control_logic() will return whatever interval you like and you can control it from your new function but returning a value from within that.

I'm slightly confused. The new function (it's called water_backup_control_logic() ) returns the latest reading_interval. Are you saying that I need to write:

Code: Select all

return water_backup_control_logic()

instead of:

Code: Select all

return (0)

I'm not sure how that works.

I'm having a slightly bad autism day so if that's not very clear I shall clarify it later

Hope you feel better soon. I pushed a version of my new function earlier today, but it doesn't include the above.

[hamishmb]

Your code looks okay, other than that you don't turn the butts pump on at most levels - you probably want to check if there's room and turn the pump on if possible at levels other than 600mm, otherwise it will never start pumping. This might be your intention of course, but I assume you'd rather have as little water in there as possible, rather than doing nothing until potentially 600mm are in the sump?

You also need to check the float status at each level where the butts pump might be on to make sure it is turned off if the wendy butts are overflowing.

We also don't necessarily need logger.warning()s for the low levels in night mode, as the idea is to get the sump empty - this is what we want, not a bad situation to avoid. Perhaps change these to logger.info()s.

Yes, you'll need

Code: Select all

return water_backup_control_logic()

, otherwise it will set the interval to 0, which might have disastrous results. Namely, probably causing 100% CPU load as everything tries to run as fast as possible.

The way this works, is that when water_backup_control_logic() is run, that statement will evaluate to whatever is returned by water_backup_control_logic(). So, if water_backup_control_logic() returns 15, the statement in sumppi_control_logic() will become:

Code: Select all

return 15

Or if your water_backup_control_logic() returns 60, it will become:

Code: Select all

return 60

Does that help?

Finally,

Code: Select all

if (hour >= 16 or hour <= 9):

Should probably be:

Code: Select all

if (hour >= 16 or hour <= 8):

Otherwise night mode will continue to run until 10AM, as presumably 9:59:59 UTC will return an "hour" of 9. Unless you wanted it to run until 10AM that is.

EDIT: I have realised that that's quite a lot of comments, I do not intend to be discouraging, I think you've done a good job. And thanks, I'm getting there with feeling better.

[TerryJC]

Hamish,

Thanks for your comments and your explanation of how the return works. I'll probably do the updates later today or tomorrow morning.

I see the logic of stopping Night Mode at 9 am (local) instead of 10 am. I suppose it depends on how long it will take to pump the water back. Also if it's stopped at 9 am, then the Duty Manager can view the situation at 9:30 with enough time to top-up the system if the backed up water turns out to be insufficient. I'll speak to Penri about what he thinks is best.

[hamishmb]

No problem.

I haven't gotten the VMs up to date and sorted yet, so there is no rush.