-
Notifications
You must be signed in to change notification settings - Fork 1k
Add support for MEI device identification encoding respecting PDU size and repeated Object IDs in the device identification #293
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
Add support for MEI device identification encoding respecting PDU size and repeated Object IDs in the device identification #293
Conversation
|
@chintal would you mind adding an example (both for server/client) to showcase this change ? Also please update unit tests for the same. |
|
@dhoomakethu The change is currently only on the client side. The change on the server side would be relatively more intrusive, but I shall make it anywajy and perhaps back out the changes later on if there are concerns. I will look at the unit tests and examples. I think the unit tests should be manageable, but I'm not entirely certain how / where the examples would go. Any example would presumably need both a server and a client to showcase the change? Do you have recommendations of a current example or pair of examples which I can use as a starting point for one? As as aside, now that I look at the server code for MEI, it looks like it would not actually be compatible with ModbusRTU, specifically due to the RTU frame size limit of 256 bytes. Is this on purpose? |
|
@chintal you can check the files synchronous_server and synchronous_client |
24ac264 to
d990165
Compare
|
@dhoomakethu I have added the example you requested. I have also put in the superstructure to handle the case of splitting the device information across multiple responses, but see below. |
pymodbus/mei_message.py
Outdated
| self.read_code, self.conformity) | ||
| # TODO By the time the code gets here, self.space_left should be set | ||
| # to the length of the largest allowable PDU for the transport and | ||
| # framer, ie, 252 for RTU. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This thing needs to be done before split responses will actually work.
|
@dhoomakethu This PR is done. It should fix the ReadDeviceInformationResponse for Servers and includes an example for how a client should handle it. As far I can tell it should be ready to merge and should cause no change in behavior for existing user code. I'm still not especially comfortable with adding a separate 'request' for the client which wraps multiple individual modbus transactions. This might create a number of other special cases which will be annoying to handle and to maintain. If you have a preexisting structure which already wraps multiple transactions within the codebase, you may use that to create another such instance if you desire it. |
…han one PDU to pack.
2902b99 to
bcfca86
Compare
This PR allows parsing of some kinds of non-standard MEI device information responses. In particular, if a device (modbus server) reply to an MEI device information request (0x2B 0x0E) includes multiple objects with the same ID. I am uncertain about whether any current devices do this, though given the fact that I have had good reason to add such behavior in my devices and the likes of #47, I would not be surprised if there are indeed a few which do so.
This change included in this PR will cause no change in the behavior of any existing code with standard devices, and continues to return a dict with the object IDs as keys. However, in case an object ID is encountered twice, the value in the dictionary against that object ID (and only that object ID) is turned into a list. The objects in the list appear in the order in which they are encountered in the response. The performance cost to this approach should be minimal to the point of being negligible.
As an example of where I use such a mechanism, I dedicate one object ID to storing version numbers of included libraries in a semi-descriptive string. As multiple libraries are linked into the firmware, each adds a device information object with it's version string. Another example where I intend to use such behavior is to store per-channel calibration information in a client-readable format, in instances where the number of channels is not known before hand.
I do currently have a functioning workaround, which subclasses ReadDeviceInformationResponse and makes the necessary changes. However, changing the Response parser and maintaining it separately does take a little more effort than I am quite comfortable with, given that ModbusSerialClient needs to subclassed as well and ClientDecoder needs to be replicated entirely.