espressif / esp-modbus

ESP-Modbus - the officially suppported library for Modbus protocol (serial RS485 + TCP over WiFi or Ethernet).
Apache License 2.0
107 stars 49 forks source link

Write Single Register Configuration (IDFGH-11919) #48

Open dmglogowskiOS opened 8 months ago

dmglogowskiOS commented 8 months ago

Hello!

We are running into issues trying to understand how to use this component properly, specifically in regards on how to write to a single holding register.

Our Slave Device does not currently support writing multiple registers using function code 0x10 as this functionality wasn't yet needed as up till now we used the function code 0x06 to write to a single Holding Register.

We know that we could manually write the request but this is more error prone in our opinion.

Therefore the question, is it possible to configure a member of our mb_parameter_descriptor_t[] to use Function code 0x06 instead of 0x10?

alisitsyn commented 8 months ago

Hello @dmglogowskiOS,

Thank you for the issue. Different slaves have different list of supported commands. There is no flexible way to configure which command to use for write request for now. The stack can not cover all possible options but provides the flexible way to use custom commands as described above. You can refer to example on how to send custom request to the slave with the command 0x06. This solution is mentioned in the documentation . Please let me know if you need more information.

Thanks.

dmglogowskiOS commented 8 months ago

Hi,

thank you for the help and I was able to create a function for our device temporarily.

However after looking through the code and thinking about things, I can imagine a few ways to actually implement this functionality without that much hassle...

alisitsyn commented 8 months ago

Hi,

Custom implementation is also possible. I also have the ideas on how to accomplish this but currently I am busy with some other things. Please feel free to share your ideas. I will try to review as soon as possible from my side. It is important for me to provide more convenient way for users related to this feature.

Thanks.

dmglogowskiOS commented 8 months ago

Hi,

I will get into working on that either later today or sometime later this week. Either way, no problem.

alisitsyn commented 8 months ago

One important comment: I have an requirement for the project is that the external API interface for v1.0.x of esp-modbus should not be changed. This includes the format of Data dictionary. The changes of interface for v1.0.x will not be applied. This is why I did not change this directly but added the workaround for this as an additional function which was the part of the interface API since esp-idf 4.0. However, the changes you are working on can be implemented in esp-modbus v2.0.0-beta.

dmglogowskiOS commented 8 months ago

Entirely understandable, my approach would likely end up adding to the Data Dictionary and thus causing this requirement to break.

However I am of the opinion that sometimes you gotta break compatibility, document it and be fine with it.

alisitsyn commented 8 months ago

However I am of the opinion that sometimes you gotta break compatibility, document it and be fine with it.

I have got your opinion and will try to address this when possible.

dmglogowskiOS commented 8 months ago

I think I actually found a way that wouldn't break compatibility but still allow commands to be configurable when wanted while the current behavior stays default.

DomiMartinGlogi commented 8 months ago

PR is opened regarding this

alisitsyn commented 8 months ago

The PR will not be merged due to the breaking changes. For v1.0.x I recommend to use the existing compatibility way of sending the custom requests with command 0x06 which works just fine. See the documentation and examples mentioned above.

DomiMartinGlogi commented 8 months ago

As far as I am aware the changes shouldn't be breaking, however I would love to get more detailed information as to why you think they are.

Either way, implementing these changes should be, imo, done for the next major version.

alisitsyn commented 4 months ago

@ DomiMartinGlogi ,

Sorry for even very late response. Unfortunately your code changes the interface and will cause the maintaining issues later. I have other idea to implement this using compatible way. I will address this when possible.