Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
zalexua opened this issue Apr 17, 2022 · 3 comments

Comments

@zalexua
Copy link
Contributor

zalexua commented Apr 17, 2022

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
Copy link
Contributor Author

zalexua commented Apr 18, 2022

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
Copy link
Owner

ShayBox commented Apr 23, 2022

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
Copy link
Contributor Author

zalexua commented Apr 23, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants