libmodbus icon indicating copy to clipboard operation
libmodbus copied to clipboard

ESP-IDF support

Open fedepell opened this issue 1 year ago • 12 comments

Adds support for serial for ESP-IDF and introduces a document on how to use it in that environment.

Assumes to be merged after #741 (I will rebase accordingly in case).

fedepell avatar May 20 '24 05:05 fedepell

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient...

cla-bot[bot] avatar May 20 '24 05:05 cla-bot[bot]

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient...

cla-bot[bot] avatar Jun 29 '24 02:06 cla-bot[bot]

sorry, i overlooked this !

diplfranzhoepfinger avatar Aug 19 '24 10:08 diplfranzhoepfinger

sorry, i overlooked this !

No problem :) Sorry for my delay now, was on a short vacation ;)

I hope we can get this (ESP support in general, in some variant) in as it may be very handy to have it mainstreamed. Also I've done a lot of testing with my code (both serial and TCP) and works like a charm (while the official internal library has a lot of limitations, ie. trivially missing possibility to have multiple connections which is just addressed in the beta).

fedepell avatar Aug 23 '24 04:08 fedepell

@fedepell Thanks for your efforts with the ESP-IDF platform support!

I've tried to use your implementation as well, but stumbled upon some issues and wanted to asked whether you can confirm that: Using the ESP-IDF UART VFS with Modbus RTU, the uart_read() function does return early on some special characters (0x0A, 0x0D) depending on the line endings, which then creates either CRC errors or incomplete messages thereafter:

2024-10-07 07:23:09 Waiting for a confirmation...
2024-10-07 07:23:09 <47><03><02><00><0D><0A><B1>
2024-10-07 07:23:09 ERROR CRC received 0xB10A != CRC calculated 0x4EF0
2024-10-07 07:23:10 [47][03][9C][98][00][01][25][13]
2024-10-07 07:23:10 Waiting for a confirmation...
2024-10-07 07:23:10 <8C><47><03><02><00>
2024-10-07 07:23:10 Request for slave 140 ignored (not 71)
2024-10-07 07:23:10 The responding slave 140 isn't the requested slave 71

I've found a ticket on the ESP-IDF project where this issue with VFS UART read() behaviour is being addressed: https://github.com/espressif/esp-idf/issues/14155

Would be interested in your opinion about that and whether you've seen similar issues.

robin-twice avatar Oct 07 '24 07:10 robin-twice

Could you rebase your draft PR against master branch, please?

stephane avatar Oct 22 '24 12:10 stephane

Could you rebase your draft PR against master branch, please?

Rebased on master and squashed commits to one!

fedepell avatar Oct 23 '24 02:10 fedepell

@robin-twice : sorry for the late response, very busy period :(

I've noticed some seldom "bad crc" messages, but didn't investigate in big detail as I saw (I did many long term tests and kept statistics of errors by type and so on) the same (very low) amount, actually even slightly higher, using the ESP native modbus library (which may suffer from the same problem if it reads data same way?) which I compared to (so I assumed it was just my lab-grade cabling or the 485 level adapter I have flying).

I'll try to give it a deeper look ASAP, hopefully this weekend or next week!

fedepell avatar Oct 23 '24 02:10 fedepell

I just pushed a few moments ago two little changes:

  • use uart_vfs_dev_use_driver instead of esp_vfs_dev_uart_use_driver as the latter is deprecated and resulted in a warning
  • Add in the docs also a note of having possibly (if console UART is disabled, something I faced today ;-) ) to explicitly initialize the VFS in your code if needed (I would not put this inside libmodbus as it is a system wide decision) with uart_vfs_dev_register (see: https://github.com/stephane/libmodbus/blob/a4eb887b6cb1926efde97ff6c8db913429c11986/src/esp-idf/README.md?plain=1#L54)

fedepell avatar Oct 30 '24 06:10 fedepell

@fedepell Thanks for your efforts with the ESP-IDF platform support!

I've tried to use your implementation as well, but stumbled upon some issues and wanted to asked whether you can confirm that: Using the ESP-IDF UART VFS with Modbus RTU, the uart_read() function does return early on some special characters (0x0A, 0x0D) depending on the line endings, which then creates either CRC errors or incomplete messages thereafter:

2024-10-07 07:23:09 Waiting for a confirmation...
2024-10-07 07:23:09 <47><03><02><00><0D><0A><B1>
2024-10-07 07:23:09 ERROR CRC received 0xB10A != CRC calculated 0x4EF0
2024-10-07 07:23:10 [47][03][9C][98][00][01][25][13]
2024-10-07 07:23:10 Waiting for a confirmation...
2024-10-07 07:23:10 <8C><47><03><02><00>
2024-10-07 07:23:10 Request for slave 140 ignored (not 71)
2024-10-07 07:23:10 The responding slave 140 isn't the requested slave 71

I've found a ticket on the ESP-IDF project where this issue with VFS UART read() behaviour is being addressed: espressif/esp-idf#14155

Would be interested in your opinion about that and whether you've seen similar issues.

Sorry it took forever to me to test :-( ($DAYJOB priorities ;-) )

I partially confirm what you found: to reproduce I crafted requests of 10 registers (so a 0x0a gets there with no doubts) and could reproduce, the write was not going through.

I've then added (see last pushed commit) the ESP_LINE_ENDING_LF configuration (to both read and write) and that seems to be gone now. And now I don't get also that seldom errors I saw (interesting enough the same problem was in the ESP internal modbus protocol, so that mislead me :-( ).

But as I see here in libmodbus we are anyway select()-ing and read()-ing until a certain length we want (or timeout of course) so we are anyhow looping on it (as in "alternatives you have considered" section in the ticket you linked to), so I'm not sure the problem should apply?

If it should, it seems that the solution was finally merged in master (https://github.com/espressif/esp-idf/commit/92b4c62d06cf10fc731b260ffee659a904bf0860#diff-8f70148cf331b8d8293e6c953cf8d7ce6a5951cb0ceb6c087ffd9c176aabc292R172) so should be out with next release, so maybe still not worth doing further workarounds? (what do you say @stephane ?)

Many thanks for your feedback and sorry again for the very late feedback :(

fedepell avatar Nov 19 '24 06:11 fedepell

@fedepell No worries, thanks for coming back to it :) ESP_LINE_ENDING_LF did help somewhat for me, but there were still some cases where it returned early (can't remember exactly whether on 0x0A or 0x0D).

Anyway, I also agree that with https://github.com/espressif/esp-idf/issues/14155 fixed it should not be an issue anymore, I haven't tested it myself though. Maybe you could also add to the readme that it needs >= xx.xx release of ESP-IDF to properly work.

robin-twice avatar Nov 19 '24 07:11 robin-twice

@fedepell No worries, thanks for coming back to it :) ESP_LINE_ENDING_LF did help somewhat for me, but there were still some cases where it returned early (can't remember exactly whether on 0x0A or 0x0D).

Anyway, I also agree that with espressif/esp-idf#14155 fixed it should not be an issue anymore, I haven't tested it myself though. Maybe you could also add to the readme that it needs >= xx.xx release of ESP-IDF to properly work.

Added a few lines to address this in ESP specific README heads-up and a reference to the ticket, good to save possible headaches to some future user :)

fedepell avatar Nov 19 '24 10:11 fedepell