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

modbus is retrying the shit out of the RD #19

Open
stei-f opened this issue Oct 10, 2022 · 4 comments
Open

modbus is retrying the shit out of the RD #19

stei-f opened this issue Oct 10, 2022 · 4 comments

Comments

@stei-f
Copy link

stei-f commented Oct 10, 2022

I'm using this library here 24/7. I'm also power-cycling the RD on regular basis, and reconnect.

Problems I saw was, the communication was kind of slow. Reconnecting is very unreliable and sometimes lead python to die due to deep recursion.
So I added a retry limit for the modbus stuff. And suddenly nothing goes. I debugged how many retries it did take for a single command, and it was up to 600!

Ok, so I found a bug:

3 minimalmodbus.TIMEOUT = 0.5 #???
Does nothing, I guess. (?)

According to the documentation of minimalmodbus, you have to set it like this:

snipped from init:

        self.port = port
        self.address = address
        self.instrument = minimalmodbus.Instrument(port=port, slaveaddress=address)
        self.instrument.serial.baudrate = baudrate
        self.instrument.serial.timeout = 0.5                                       #<--- like this

And by doing this, this module here gets decently fast. I only getting a single retry once in a while.

By the way, what I did, when implementing a retry:

    def _read_register(self, register):
        if( self.attempt < self.retry ):
          try:
              r = self.instrument.read_register(register)
              self.attempt = 0
              return r
          except minimalmodbus.NoResponseError:
              self.attempt+=1
              return self._read_register(register)

I think you get it. The object (class) stores a retry value. The object a retry counter. Be aware that this is NOT thread safe (but the whole module is not). Also you can softlock the module. So I also added some helpers to check and reset:

    def isFailed(self):
        return self.attempt >= self.retry

    def clearRetry(self):
        self.attempt = 0

Maybe the developer likes this idea. (I just did want to get rid of the deep recursion happening.) Also by doing so, you can blind fire the thing against a none powered RD, and "estimate" from the result that it is not powered.

(If I would know how to do git in this webpage, I could do a "push" request or something. Than you could see my version, and take over what you like.)

@Baldanos
Copy link
Owner

Your bug description is correct, and your retry method sounds good to me.
I'm OK with adding your proposed changes myself but If you'd like to be credited please submit a pull request with the changes you describe.
See here for a tutorial.

@rrnicolay
Copy link

Even after the change proposed by @stei-f, I'm still getting high latency times to read a single register. Is this normal?
I'm measuring ~216ms to read a single register (Output voltage).
I'm using RD6006 in v1.41 connected to a Raspberry PI 4.

@stei-f
Copy link
Author

stei-f commented Feb 24, 2023

OK, had a different text here, but decided to measure it again.
So I get rountdtrips on modbus with my rd6024 reading several registers in one shot(see text below) between 40ms and occasional spikes to 500ms. It seems to group around +120ms - 90ms i would say. So your number seems only slightly slower than what I get (if they even are, depending on how you measured). I just did the crude time.time test, don't know how accurate this is on a RPi3.

What you can do is request a range of registers, and it will take the same time as requesting one register.
But you have to extend the library for this, or code your way around it. So if you need a few values and they are close to each other, try reading the range from first to last register. (what he did in the "status" method)

@rrnicolay
Copy link

@stei-f I did measure using time.time as well.
Yes, reading a range of registers is much master. But I need to read only the "Output Voltage" register as fast as I can.
I think the problem is with the rd6006, because the round trip time feature from minimalmodbus says that almost all the time was spend waiting for the psu to reply (the round trip time is almost equal to the total time of the reading).
Maybe that time is enough for me, but I was expecting that the reading would occur much faster.

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

3 participants