ShayBox / Riden

A python library for Riden RD power supplies
MIT License
48 stars 13 forks source link

get* functions actually physically re-read registers if bulk read values were 0 #6

Open zalexua opened 2 years ago

zalexua commented 2 years ago

Add debug (2ndline) to this function:

    def read(self, register: int, length: int = 1) -> int or tuple:
        print(f"reg: {register}: len: {length}")

observe this output when running "riden" (power output if OFF):

reg: 0: len: 4
reg: 1: len: 1
reg: 4: len: 16
reg: 4: len: 1
reg: 6: len: 1
reg: 10: len: 1
reg: 11: len: 1
reg: 13: len: 1
reg: 15: len: 1
reg: 16: len: 1
reg: 17: len: 1
reg: 18: len: 1
reg: 19: len: 1
reg: 32: len: 10
reg: 32: len: 1
reg: 38: len: 1
reg: 39: len: 1
reg: 40: len: 1
reg: 41: len: 1

while we should expect only these, for calls "init" and "update":

reg: 4: len: 16
reg: 32: len: 10

As a proof, when I turn power output ON (which is R.V_OUT), then "reg: 10: len: 1" line is not printed.

those single registers reading happened for registers which have 0 (zero) as current value. so construction:

_v_out = _v_out or self.read(R.V_OUT)

works incorrectly when function input value is set to 0 (zero). While approach used before refactoring works correctly:


if _v_out is None:
    _v_out = self.read(R.V_OUT)
zalexua commented 2 years ago

proof on more way

# python
Python 3.9.7 (default, Sep 10 2021, 14:59:43)
[GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> v=0
>>> v = v or print(v);
0
>>> v=3
>>> v = v or print(v);
>>>
ShayBox commented 2 years ago

Hmm that's unfortunate, I am undecided if I should go back to the double line approach

if _v_out is None:
    _v_out = self.read(R.V_OUT)

or replace or with if not None else, which turns

_v_out = _v_out or self.read(R.V_OUT)

into

_v_out = _v_out if not None else self.read(R.V_OUT)

It would reduce line count, but it's less readable

zalexua commented 2 years ago

I actually already switched to 2-lines style already in my local branch, fixed all places already. Should do a pull request soon, with some other additions.So, you could just wait, if that approach will be acceptable by you.