espressif / esp-modbus

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

How to lock access to slave reg areas while writing? (IDFGH-12788) #61

Open WolfspiritM opened 2 months ago

WolfspiritM commented 2 months ago

Hi,

I'm using the esp-modbus component as a slave. How can I make sure that no "half-written" data is read?

According to the docs the changing code should be wrapped in a:

portENTER_CRITICAL(...);
portEXIT_CRITICAL(...);

However the examples declare the static mutex at the top of the example file. So, as far as I understand, it won't prevent the esp-modbus task from trying to read the reg area.

I thought maybe it's cause it disables interrupts preventing request from arriving, but looking up critical sections HERE it says that it only disables interrupts for the current core. However per default the esp-modbus task is pinned to cpu0 while the application per default runs on cpu1 (esp32).

Maybe I'm missing something but does that mean to prevent accidental half written reads from happening I have to create a task on cpu0 that changes the code via a critical section (or pin esp-modbus to cpu1)?

Would it be possible to expose a shared mutex or is there any other way to synchronize?

alisitsyn commented 2 months ago

Hi @WolfspiritM ,

However the examples declare the static mutex at the top of the example file.

At the top of example file is the spin lock declared, not the static mutex See the [documentation]([spin locks]https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/freertos_idf.html#critical-sections)

* Spinlocks are of portMUX_TYPE (not to be confused to FreeRTOS mutexes)

However per default the esp-modbus task is pinned to cpu0 while the application per default runs on cpu1 (esp32).

As per kconfig the default affinity for the main_task is CPU0. The default value for the esp-modbus tasks is also the CPU0. So, the critical section guaranties that the value is not "half-written".

Would it be possible to expose a shared mutex or is there any other way to synchronize?

The v1 stack does the synchronization this way, not perfect but works and the API will not be changed. The new version of Modbus v2 uses semaphore-mutex to lock access to shared data and has appropriate slave API functions, master API.

Please let me know if you have other comments.

WolfspiritM commented 2 months ago

Hi @alisitsyn Thank you very much for the quick response!

At the top of example file is the spin lock declared, not the static mutex

Calling it "mutex" was a mistake from me yes, I tried to read into critical sections and spin locks before and as far as I understand it is similar to a mutex in the way that it blocks access from both cores IF they enter the same spin lock. As it's only defined at the top of the sample and not used by esp-modbus the spinlock part isn't really what prevents reads while writing, right?

As per kconfig the default affinity for the main_task is CPU0. The default value for the esp-modbus tasks is also the CPU0. So, the critical section guaranties that the value is not "half-written".

Somehow I was confused cause the CPU1 core is the "app" core but obviously that's not the case anymore. Thanks for the clarification.

So if I understand correctly what I said in my first post is kind of true: I need to ensure that I change the registers from the same core that also has the esp-modbus task running AND to make sure the esp-modbus doesn't "take over" while my change happens I enter the critical section, right?

alisitsyn commented 2 months ago

@WolfspiritM ,

So if I understand correctly what I said in my first post is kind of true: I need to ensure that I change the registers from the same core that also has the esp-modbus task running AND to make sure the esp-modbus doesn't "take over" while my change happens I enter the critical section, right?

It is kind of true. When the app task on core0 enters the critical section this core will disable interrupts, and will then take the spinlock and enter the critical section. The other core1 is unaffected at this point, unless it enters its own critical section and attempts to take the same spinlock. In that case it will spin until the lock is released.