Flowmeter #220
Flowmeter #220
Conversation
|
Can you please comment a bit on the changes? To me it looks like a lot is simplyfied (most of the code is removed), but it still works? |
|
ping @matthias-schuessler |
|
I can't really comment on this, because this branch is entirely Antonios and Thomas work. I never worked with this branch and our current module qc environment does not use the changes in it. I can look into this. |
|
Yes, please have a look. In the longterm we want to use the flowmeter for our setup and it would be nice to check if the code changes actually work. Thanks! :) |
matthias-schuessler
force-pushed
the
flowmeter
branch
from
9f2edef
to
2c7b679
Compare
matthias-schuessler
force-pushed
the
flowmeter
branch
from
2c7b679
to
b2ef266
Compare
author AntonioT7 <[email protected]> 1669815880 +0100 committer Matthias <[email protected]> 1715085603 +0200 new julabo fp50 new julabo fp50 Update julabo_FP50.yaml new julabo fp50
julabo FP50: update
matthias-schuessler
force-pushed
the
flowmeter
branch
from
b2ef266
to
254ab29
Compare
|
@matthias-schuessler Why do you always force-push? If it does not work with a regular push, your local history is messed up. |
|
@cbespin I did this, since I just performed a rebase of flowmeter onto the current version of master (basil release 3.3.0). From what I gathered from documentation, a force-push is necessary after a rebase since the idea behind rebase is to change the branch history. |
The flowmeter changes in here are working properly after rebase and can be considered ready for merge. |
|
You do not need a force-push after a rebase. This is the idea of a rebase ;) force-push overwrites the remote history. Always try with a normal push and if you encounter errors, deal with them first before considering a force-push. I added myself as reviewer and will take a look as soon as possible |
|
|
||
| def set_setpoint(self, value): | ||
| cmd = [1, 1, 0x21, (value >> 8) & 0xFF, value & 0xFF] | ||
| self.write(cmd) | ||
| """value range from 0 - 32000 |
There was a problem hiding this comment.
Is the range exactly 32000 or 32767 aka 2^15? The command seems to allow 2^16 actually? Iirc, there is a large range that does not influence the effective the setting, is it this one? Not sure if we should print a warning or info or ignore it, since the user should not care? @YannickDieter
There was a problem hiding this comment.
I heard @SilasM2001, you know more about that?
There was a problem hiding this comment.
I worked with the new version in the flowmeter branch. I can not commend on the previous code. The manual says setpoint has a range of 0 to 32000, with 32000 being 100%. In my experience the range works fine. The module testing setup of Matthias uses a large range (1000-25000 or so, if i recall correctly) and i did not observe a large range that does not influence the effective setting.
| def set_control_mode(self, value): | ||
| answer_in_hex = ret[11:] # read from the 11th digits to translate what point is set | ||
| answer = int(answer_in_hex, 16) | ||
| return answer |
There was a problem hiding this comment.
I think this might even be less "confusing" if you do answer = int(ret[11:], 16) # convert reply from hex to integer
| cmd = [4, 1, 0x21, 1, 0x20] | ||
| self.write(cmd) | ||
| answer_in_hex = ret[11:] # read from the 11th digits to translate what mode is on | ||
| answer = int(answer_in_hex, 16) |
| logger.debug("ELFLOW.get_valve_output() ret error ret=%s" % str(ret)) | ||
| return -1 | ||
| answer_in_hex = ret[11:] # read from the 11th digits to translate what the capacity is | ||
| cap_100 = struct.unpack('!f', bytes.fromhex(answer_in_hex))[0] |
There was a problem hiding this comment.
cap_100 is not really conclusive. Is this the maximum capacity in percent? Maybe add a comment or find a more descriptive variable name
No description provided.