Pre-merge river system questions

[hamishmb]

So as we approach the point where we need to merge our code back into master, I've been going through the use-database branch and cleaning things up as much as possible to ease the process. We will need a merge plan, I think, to try and ensure it goes smoothly, but before we get there I have some questions about the system. I figured it's best to make a separate topic for these rather than create lots of spam.

Potentially any of you might know the answers to these, because a lot of them concern integration of the system as a whole and how it functions in the real world.

Question #1: Does anyone know exactly what revision of the code is currently running on the pis? As we prepare to merge, I think we'll want to gradually bring the system forward in time, rather than in a big leap (thoughts on this?). If anyone knows exactly what commit/revision the system is currently running, it would be invaluable for this.

Question #2: Back in October 2019, I made a slight modification to the code to increase the positional tolerance of the gate valves to 2% to try and stop them from hunting. There's still a note in the code about seeing if this worked or not. I'm pretty sure this code was deployed to WMT, so has anyone observed the gate valve hunting?

Question #3: Terry, in Tools/devicemanagement.py, at the end of the get_compensated_probe_voltages() method adapted from your code, there's a note from me asking about whether a particular if ladder is needed or not, but I don't understand the code well enough to be certain. Can you shed any light on this?

There will be more questions to add, but I figure I might as well ask in manageable chunks. This is probably a good place for others to ask questions on this too

[TerryJC]

Question #1: Does anyone know exactly what revision of the code is currently running on the pis? As we prepare to merge, I think we'll want to gradually bring the system forward in time, rather than in a big leap (thoughts on this?). If anyone knows exactly what commit/revision the system is currently running, it would be invaluable for this.

I don't know the answer to this. However, does git add any kind of index number to the files, so that we can find out simply by looking at it. I/we can currently log in to the WMT-Webserver Pi using SSH because Penri put in a Port Forward to that Pi. From there we can get to any Pi running on the system. If there is anything in the code that will help us find the current version that would do it.

(If there isn't there should be; we had that on SourceSafe around 15 to 20 years ago, by automatically embedding the SourceSafe Version in to the Header comments.)

Question #2: Back in October 2019, I made a slight modification to the code to increase the positional tolerance of the gate valves to 2% to try and stop them from hunting. There's still a note in the code about seeing if this worked or not. I'm pretty sure this code was deployed to WMT, so has anyone observed the gate valve hunting?

No. But see above. If you tell me where in the code this note is I'll have a look.

Question #3: Terry, in Tools/devicemanagement.py, at the end of the get_compensated_probe_voltages() method adapted from your code, there's a note from me asking about whether a particular if ladder is needed or not, but I don't understand the code well enough to be certain. Can you shed any light on this?

I'm assuming you mean the entry 'Will this ever happen? It seems impossible to me.' ? It's been a while, but I think that this might have been left over from an earlier version and intended to trap if the level is between sensors. The truth is I'm not sure.

[hamishmb]

I don't know the answer to this. However, does git add any kind of index number to the files, so that we can find out simply by looking at it. I/we can currently log in to the WMT-Webserver Pi using SSH because Penri put in a Port Forward to that Pi. From there we can get to any Pi running on the system. If there is anything in the code that will help us find the current version that would do it.

(If there isn't there should be; we had that on SourceSafe around 15 to 20 years ago, by automatically embedding the SourceSafe Version in to the Header comments.)

Unfortunately, I don't think git does this, as far as I can tell from a quick search. I'm going to do a deeper search for this now - if we have to enable it it's worth doing in future anyway. I was just hoping that someone had kept a record.

(#2) No. But see above. If you tell me where in the code this note is I'll have a look.

This is in the calculate_limits() method in devicemanagement.py, around line 363 in the use-database branch. This hasn't changed in that branch, so I imagine it's a similar line number in the WMT-deployed code.

I'm assuming you mean the entry 'Will this ever happen? It seems impossible to me.' ? It's been a while, but I think that this might have been left over from an earlier version and intended to trap if the level is between sensors. The truth is I'm not sure.

Okay. What should we do then? I don't know if it's important enough to be worth removing it (the if ladder) and potentially breaking code that seems to work just fine, so should we just remove the note? However, it's also not good if we don't know whether we need it (the if ladder) or not.

[TerryJC]

This is in the calculate_limits() method in devicemanagement.py, around line 363 in the use-database branch. This hasn't changed in that branch, so I imagine it's a similar line number in the WMT-deployed code.

This is what I found in the V4 Gate Valve version:

Code: Select all

               #TODO Tolerance increased to 2%, see if hunting stops.
                self.high_limit = self.valve.min_open + 2

I found the same lines in Sump Pi and Wendy Butts Pi.

Okay. What should we do then? I don't know if it's important enough to be worth removing it and potentially breaking code that seems to work just fine, so should we just remove the note? However, it's also not good if we don't know whether we need it or not.

I agree, but we're talking about analogue values here and there may be a scenario where the values are very close together with insufficient difference to produce a minimum. Rather than remove the comment, perhaps we should simply change it to say something like 'Catchall for corner cases where a minimum cannot be determined'.

[hamishmb]

This is what I found in the V4 Gate Valve version:

Code: Select all

#TODO Tolerance increased to 2%, see if hunting stops.
self.high_limit = self.valve.min_open + 2

I found the same lines in Sump Pi and Wendy Butts Pi.

Ah excellent, that's the same as what I was looking at. I guess that's all fine then and I can remove the note.

I agree, but we're talking about analogue values here and there may be a scenario where the values are very close together with insufficient difference to produce a minimum. Rather than remove the comment, perhaps we should simply change it to say something like 'Catchall for corner cases where a minimum cannot be determined'.

Sounds good. Looking at the code again, I think this corner case can't happen, but leaving a note there seems like a good idea.

I'll get back to you when I've figured out if git leaves some kind of signature. How do I SSH in, in the mean time? (might be useful if I need to poke around a bit).

[TerryJC]

How do I SSH in, in the mean time? (might be useful if I need to poke around a bit).

I'll let you know by a more private means

Who knows who might stumble on this?

This facility is temporary until the VPN is working properly.

[hamishmb]

(Probably) final few questions:

Question #4

I have made use some new control logic functions that take advantage of the database. Seeing as these will be replaced with Patrick's logic in the fullness of time, the work has just been porting these over to using the database. The sump pi control logic is exactly the same as the existing control logic, including the varying reading intervals, except that it uses the database instead of sockets.

The NAS control logic is as we discussed, and uses a fixed interval of 15 seconds, as this determines the tick interval.

There is also some basic generic control logic for the gate valves that watches the database in order for us to be able to control them - once deployed we will be able to open and close the Hanham and Stage valves easily. Finally, there is some generic control logic for all other sites (without valves or devices to control), which just updates the system status table to show CPU and RAM usage. This also has a fixed interval of 15 seconds.

Is using fixed reading intervals for the generic and valve control logic okay for now? It seems to work okay in my tests, but we haven't discussed this so I thought I would check. I'm not sure what else to set it to, in the absence of much data (and not wishing to reinvent Patrick's shiny new wheel).

Question #5

It is noted in devicemanagement.py that we occasionally get negative position readings from the gate valves. These are currently just thrown away and ignored, which seems to work just fine as a solution. However, did we ever figure out why this was happening and/or fix it? I guess perhaps a negative value (I think it was -1) could indicate an error from the ADC?

[TerryJC]

Question #4
Is using fixed reading intervals for the generic and valve control logic okay for now? It seems to work okay in my tests, but we haven't discussed this so I thought I would check. I'm not sure what else to set it to, in the absence of much data (and not wishing to reinvent Patrick's shiny new wheel).

I think tha this could remain as it is until the system is deployed and we have more information.

Question #4Question #5
It is noted in devicemanagement.py that we occasionally get negative position readings from the gate valves. These are currently just thrown away and ignored, which seems to work just fine as a solution. However, did we ever figure out why this was happening and/or fix it? I guess perhaps a negative value (I think it was -1) could indicate an error from the ADC?

I think we need to analyse the voltage data that comes back from the Pis that are generating that -1 response to see where the number comes from. It might well be an A/D issue, or it might be something built in to the calculation. For the moment we have another problem which I'll post about elsewhere.

[Penri]

Hello

As Terry has said to solve the problem of the -1 we really need to know what voltage the A/D is seeing.

IMG_1709.jpg (127.13 KiB) Viewed 1136 times

The linear actuator's rack (the bit that goes up and down) is mechanically coupled to both the valve and the potentiometer wiper (connected to the White wire), the A/D measures the voltage on the White wire when the Green is connected to OV and the Red connector to a reference voltage, in our case 3.3V. In the Gate Valve both 3.3V and 0V are common to the potentiometer and A/D, so in theory we should only ever get a positive voltage applied to the input of the A/D and that should be somewhere between 0V and 3.3V.

Of course, in the real world unpleasant things like noise and differential potentials between Ground (0V) points can occur so I could not totally rule out the A/D seeing a negative voltage, but if there is one it would be very small.

If its possible to capture the voltage reading when the valve returns a -1 we should get a clue as to what's going on.

Penri

[hamishmb]

Oops, I forgot about this thread after hearing about the VPN weirdness - did not mean to ignore you, Terry

I think that this could remain as it is until the system is deployed and we have more information.

Okay.

(Valve voltage comments):

Okay, makes sense both, and the voltage measuring is definitely doable. Seeing as this doesn't seem to be that much of a problem, and the software's pretty much ready to deploy now, subject to a bit more testing, shall we put this one on the back-burner for the moment? Or would you rather resolve it now?