nanoframework / Home

:house: The landing page for .NET nanoFramework repositories.
https://www.nanoframework.net
MIT License
861 stars 79 forks source link

Modbus library will skip first byte received #1476

Closed JohnMasen closed 1 month ago

JohnMasen commented 5 months ago

Library/API/IoT binding

nanoFramework.IoT.Device.ModBus

Visual Studio version

VS2022

.NET nanoFramework extension version

2022.3.0.78

Target name(s)

No response

Firmware version

No response

Device capabilities

No response

Description

Modbus document requries a 3.5 characters timing between sending and receving. Instead of wait a certain time, the library will do a dummy read as sleep function after sending a request. Here's a demo chart shows 2 different cases.

--  -- <-3.5 char timing -> <-- Data Frame -->
Case1 SendData Read dummy byte( use dummy read as sleep function) Read data  
Case2 SendData CPU work on other thread Read dummy byte( 1st byte was skpped) Read data

In case 2, the first byte was skipped. This happends when my device is running a web server as background service. It could also happends on multi-threaded application or low performance CPU device.

How to reproduce

  1. Create a modbus client project with multi-threading (example: a background web server)
  2. Connect to server and send a read hold-register request
  3. The library will return null result
  4. By observing the debug info, you can find the first byte was skipped, this triggers CRC error causes the library return null

Expected behaviour

The library should return correct server response.

Screenshots

No response

Sample project or code

No response

Aditional information

No response

josesimoes commented 5 months ago

<20ms sleeps are not manageable with nanoFramework. The next best option would be with Spin.Wait() but even with that, exact timming is not guaranteed. On the contrary, reading dummy bytes with the baud rate on the UART provides exact time because of the way it works. That's the reason why this is the current implementation used in the library.

JohnMasen commented 5 months ago

Thanks for your information. as sleep function may not be accurate enough, I'll do more investigations on this. I think there's difference between RS232 and RS485 in the nanoframework and we should have different logic for either mode. Will keey you guys updated.

JohnMasen commented 5 months ago

After some digginer, I think the best solution for now is read 3 bytes and do a pattern check ([id]-[fcode]) from first 2 bytes. What I did:

  1. Read the source code of SerialPort implementation of my board (ESP32-C3), found out it will read the underlyne UART port if RXbuffer is empty. This explains why read single byte will help do the delay, but seems this is controlled by SerialPort.ReadTimeOut property, so it's behavior is based on user settings and the minimum value is 1 ms. Reference : https://github.com/nanoframework/nf-interpreter/blob/main/targets/ESP32/_nanoCLR/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort.cpp
    hbTimeout.SetInteger((CLR_INT64)pThis[FIELD___readTimeout].NumericByRef().s4 * TIME_CONVERSION__TO_MILLISECONDS);
    NANOCLR_CHECK_HRESULT(stack.SetupTimeoutFromTicks(hbTimeout, timeoutTicks));
  2. Found System.Diagnostics.StopWatch may helpful, not tested yet
  3. Find one of my TTL to Modbus RTU (RS232) bridge device has frame-gap setting (1-100 ms) , which insert a delay between each TX/RX. my other one has fixed 5ms frame-gap.
  4. My MPPT solar controller requires 10ms frame timing

Conclution: Ideally Modbus library should have configuable frame timing to fit various use cases. however I think do the 3 bytes pattern detection should be fine for now.

josesimoes commented 5 months ago

I'm strongly advocating for the 3 (maybe 4) dummy bytes read to acomplish an "as close as possible" timming for this. About the pattern checking, let's do some testing around that as I wouldn't like to have this incur in a performance penalty which will, obviously, exist as the code is processing it. My initial though would be to go for the simplistic approac and "just" ditch the dummy bytes before start reading the actual data.

JohnMasen commented 5 months ago

Hi @josesimoes , I agree it may slow the performance, but I can't find a better solution because:

  1. Witout the pattern checking, we can't known if dummy read is skipping the actual bytes.
  2. Some device may have longer response time( or on the multi-thread host), this makes dummy read not able to handle all the cases.
JohnMasen commented 5 months ago

I think I found a solution, will create a prototype for test.

josesimoes commented 3 months ago

@JohnMasen any progress on this?

JohnMasen commented 3 months ago

Hi @josesimoes , sorry I'm working on an urgent case these days, may take a few weeks to finish. I'll back to this after that.

I'm using an internal buffer, pattern match and "fixed skip size" combination to archive performance and memory consumption. The code will read 4+n(n=bytes skipped lasttime) bytes into internal memory, then try to skip n bytes and decode message, if it fails then it fall back to the start of buffer and begin pattern match. The whole test process may be a longer than I expect. the code is already completed, testing it costs me much time, I have to send the response with different timing to cover each code path, this really takes time.

Please feel free to close the issue by now, I can create a new PR and link it back to this issue when everything is ready.

josesimoes commented 3 months ago

No worries. Take your time. Curious on how your code will look. Trust all that processing doesn't take too much time and risking missing the data start as its busy trying to process the start.

Good luck for whatever you need it and let's keep this one open for the moment to track this.

muenchris commented 2 months ago

I have the same issue on a Modbus RTU device I am using. In my case the device does not send any dummy byte before the correct sequence. My fix was to check if the "dummy" byte is the expected deviceId, and if so, I continue to parse the sequence assuming there was no dummy byte. This works very stable in my environment.

You can try this starting from line 459 in the modbusclient.cs:

              // read dummy byte (from specs: 3.5 chars time start marker, will never ever be read in a understandible manner)
                // Some Modbus Server do not use the start marker, so we need to read the first byte and see if it is the device ID we expect and if not assume its the startup marker.
                id = DataRead();

                // read device ID
                if (id != request.DeviceId)
                {
                    id = DataRead();
                }
JohnMasen commented 2 months ago

@muenchris Please feel free to submit PR for this issue. I'm working on 2 projects at same time and my bandwidht is very limited. very glad to see anyone can help on this issue.

josesimoes commented 2 months ago

@muenchris considering all the above, can you please go with a PR implementing your suggested fix so we can wrap up this one?

muenchris commented 2 months ago

Ok, will do. I tested it in my environment with two different modbus server and it worked in both cases.

JohnMasen commented 1 month ago

Thanks @muenchris . Glad to have this issue solved.