pyhys / minimalmodbus

Easy-to-use Modbus RTU and Modbus ASCII implementation for Python.
Apache License 2.0
308 stars 145 forks source link

Interest in function code 23? #4

Open KennethNielsen opened 8 years ago

KennethNielsen commented 8 years ago

Hallo minimalmodbus devs and thanks for an amazing package.

I'm currently trying to communicate with a device that uses function code 23 (which is not currently supported by minimalmodbus). Right now I'm just using pymodbus, but I would like to use minimalmodbus for this also (since we use minimalmodbus other places as well, and since pymodbus is not as easy to use and does not support Python3).

I have read through the source code and feel relative certain that I can implement function code 23 (read/write multiple registers), but before I start, I wanted to ask whether there would be an interest in including it in minimalmodbus (provided the implementation is of high enough quality)?

If there is interest, we can have a discussion about implementation and top level interface afterwards.

KennethNielsen commented 8 years ago

Any feedback on this.

pyhys commented 8 years ago

Hi Kenneth, thanks for your interest in MinimalModbus!

Yes, it would be really interesting to also support function code 23. A patch would be welcome. As I do not have access to any instrument with this capability, good test coverage would be necessary. See https://minimalmodbus.readthedocs.org/en/master/develop.html#recording-c ommunication-data-for-unittesting http://https://minimalmodbus.readthedocs.org/en/master/develop.html#rec ording-communication-data-for-unittesting and https://github.com/pyhys/minimalmodbus/blob/master/tests/test_minimalmod bus.py http://https://github.com/pyhys/minimalmodbus/blob/master/tests/test_mi nimalmodbus.py

Note however that I nowadays do not spend much time on the software, as my dayjob is my main focus. Typically MinimalModbus is released once a year or 18 months.

Best regards Jonas

<-----Ursprungligt Meddelande----->

    From: Kenneth Nielsen [notifications@github.com]

Sent: 24/2/2016 10:50:55 AM To: minimalmodbus@noreply.github.com Subject: Re: [minimalmodbus] Interest in function code 23? (#4)

Any feedback on this.

Reply to this email directly or view it on GitHub https://github.com/pyhys/minimalmodbus/issues/4#issuecomment-188169555 . https://github.com/notifications/beacon/ABg1jiyXPRGkt-AAKJmCGIHoA09Not9 Cks5pnXR_gaJpZM4HRTRM.gif

KennethNielsen commented 8 years ago

Hi Jonas

Yes, it would be really interesting to also support function code 23.

That's great to hear.

As I do not have access to any instrument with this capability, good test coverage would be necessary.

Naturally. I'll read the references and work from there.

Note however that I nowadays do not spend much time on the software, as my dayjob is my main focus. Typically MinimalModbus is released once a year or 18 months.

That's just fine. I'll probably take me some time to implement as well, since I will have to do it as other work allows. I also don't mind it that we can use my branch until a release happens, I just didn't want to do it forever ;) I might also be able to help with some of the work pertaining to a release.

I do have one small patch that I was hoping to get it in before I start this, so I hope you will have time to look at a small pull request soonish.

Regards Kenneth

KennethNielsen commented 7 years ago

@pyhys I am working on this currently. A question of preferred design has popped up. Functioncode 23 makes it possible to write and read registers in the same command (write first and read afterwards). The current design of minimalmodbus is centered around read and write as separate operations. So the question is how to attack this:

  1. Implement a complete new set of methods for each of the types i.e. read_write_registers, read_write_float etc. This could be done, but of course in principle there is nothing wrong with writing and read registers where the data is of different type.
  2. Implement one generic read_write method, in which it is possible to provide types as arguments
  3. Ignore that functioncode 23 allows this read and write in one step and just expand the existing separate read and write functions

I tend for lean towards 3 for a couple of different reasons:

A. It is the least invasive in the code. 1 and 2 will both require changes to call signatures all the way down the stack and at least in _genericCommand, which is the entry point for everything, which makes me a little uneasy. B. It avoids expanding the nice and simple interface that minimalmodbus has C. I really only need read (selfish reason I know, but still)

What are your thoughts on this?

pyhys commented 5 years ago

Hi, yes I agree that 3 is the preferred option right now, to use it for reading those instruments that not supports reading with some other function codes.

Unfortunately, according to the standard there needs to be at least one read and one write for each call of function code 23. I do not know how to work around that.

raul-openai commented 4 years ago

Has there been any progress whatsoever on this? I am also interested in function 23 working.

Thanks!

KennethNielsen commented 4 years ago

Hi @pyhys and @raul-openai

I apologize for dropping of the radar on this one. The status is that I did some work on it at my previous work, but was unable to find the time to record traces and implement the required tests (that I fully support should be there BTW). I've used minimalmodbus multiple time, so I'm sorry I wasn't able to do a better job of getting an improvement upstreamed. In the meantime, I found a new job, so I no longer have access to the equipment. For these reason the best that I can do is to try and do proper hand-off.

I have implemented some support for function code 23 in read_float and read_string in the master branch in this fork: https://github.com/CINF/minimalmodbus. Unfortunately, as pointed out by @pyhys, according to the spec it is not actually possible to separate the read from the write, and so the way that I implemented it, which is setting the number of bytes to write to 0, is non-spec compliant, and therefore should not be merged. I think at the time I either missed it in the spec or deliberately ignored it because it worked with the piece of equipment I had. @raul-openai might work for you as well, as a bad workaround in the mean time.

Actually given this information, I think, in hindsight, that the best way to attack the problem for minimalmodbus is actually option 2 mentioned above, but as I said, I don't have the option to work on it anymore. If I get some equipment at my new job that supports function code 23 I might come back and contribute.

raul-openai commented 4 years ago

Hey @KennethNielsen, thank you for the through response!

I agree that option 2 would be better. In my case I would like to communicate at high frequency. I assume that option 3 causes essentially 2 operations (write + read), which can double communication time, so a single write-read operation would be a great benefit here.

I see, I didn't know you patched it to just read on 23. I would like to write and read indeed, so the patch won't be useful for me.

Unfortunately, I have tried a different library that supports function 23 and I will be switching to that one instead, so I won't get around to expand minimalmodbus.

Thank you for the info, though!