Skip to content

ESP32 MODBUS RTU Question #94

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

Closed
zekageri opened this issue Dec 11, 2020 · 19 comments
Closed

ESP32 MODBUS RTU Question #94

zekageri opened this issue Dec 11, 2020 · 19 comments

Comments

@zekageri
Copy link

zekageri commented Dec 11, 2020

Hi! First of all this is an amazing lib and thank you for all of this.

Is it possible to be blocking reads and writes all the time? Instead of callback return the result right away?

Currently i'm using it like this:

bool SaveInput(Modbus::ResultCode event, uint16_t transactionId, void* data) {
    if(event == mb.EX_SUCCESS){
        uint16_t PIN = Detect_Bits_Change(Expander_Info[ADDRESS_COUNTER]->Inputs[0],EXP_Input_Buffer[0]);
        if(bitRead(EXP_Input_Buffer[0],PIN)){
            Search_For_PIN(PIN,mb.eventSource());
        }
        Expander_Info[ADDRESS_COUNTER]->Inputs[0]   = EXP_Input_Buffer[0];
        Expander_Info[ADDRESS_COUNTER]->Address     = mb.eventSource();
        return true;
    }else{
        return false;
    }
}


static const inline void Read_Input_Registers(){
    if (!mb.slave()) {
        if(Expander_Info[ADDRESS_COUNTER] != 0){
            xSemaphoreTake(xMutex, portMAX_DELAY);
            mb.readHreg(Expander_Info[ADDRESS_COUNTER]->Address,Input_Reg,EXP_Input_Buffer,1,SaveInput);
            while (mb.slave()){mb.task();}
            xSemaphoreGive(xMutex);
        }
    }
    ADDRESS_COUNTER++;
    if(ADDRESS_COUNTER == MAX_EXPANDER){ADDRESS_COUNTER = 0;}
}

As you can probably see i have to wait while (mb.slave()){mb.task();}.

Instead i would do something like this:

static const inline void Read_Input_Registers(){
uint16_t EXP_Input_Buffer[2];
mb.readHreg(Expander_Info[ADDRESS_COUNTER]->Address,Input_Reg,EXP_Input_Buffer,1); 
/* NOTICE THERE ARE NO CALLBACK SPECIFIED! 
   PROCESS THE EXP_Input_Buffer[0] value!
*/

}
@zekageri
Copy link
Author

The thing is that i have more continously running tasks because there are a lot of devices on the bus.
And i solve this problem by passing a semaphore between tasks. The Read_Inpu_Registers() function is inside the main modbus tasks and it has to run no matter what. But if other task wants to use the bus, it has to take a semaphore like this:

inline void Read_Buttons(){
                xSemaphoreTake(xMutex, portMAX_DELAY);
                vTaskDelay(5);
                if (!mb.slave()) {
                    mb.readHreg(Address,0x0000,Input_Buffer,2,ThermosFallBack);
                    while (mb.slave()){mb.task();}
                }
                vTaskDelay(5);
                xSemaphoreGive(xMutex);
            }

I need that vTaskDelay(5); because the modbus is busy if the function takes the semaphore and try to write or read immidiatelly.

But this non blocking callback thing is a little bit hacky for me because i have to write separate callback for every interact inside tasks

@emelianov
Copy link
Owner

emelianov commented Dec 11, 2020

Hello. Thanks.
I don't think it's good idea to implement blocking call inside the library. The reason is that in case if slave is not answering call will block execution for a second (default timeout value). Yes, it's not a problem if you are using multithreading ESP32 code, but the library is for generic Arduino. So the library API actively forces using async calls.
As for you case, why do not create and use own wrapper functions and always use them to access Modbus from your code?
Something like

Modbus::ResultCode err;
bool resCallback(Modbus::ResultCode event, uint16_t, void*) {
    err = event;
}
Modbus::ResultCode readSync(uint16_t start, uint16_t num, uint16_t* buf) {
  xSemaphoreTake(xMutex, portMAX_DELAY);
  if (mb.slave())
    return Modbus::EX_GENERAL_FAILURE; // Normally is impossible situation
  mb.readHreg(Address, start, buf, num, resCallback);
  while (mb.slave()) {
    //vTaskDelay(1);  // Probably it'll be reasonable in some cases
    mb.task();
  }
  Modbus::ResultCode res = err;
  xSemaphoreGive(xMutex);
  return res;
}

void main() {
//..
  if (readSync(..) == Modbus::EX_SUCCESS) {
    //ok
  } else {
    //error
  }
//..
}

@zekageri
Copy link
Author

Thank you for the response. I modified the default timeout to 5ms.
I will investigate this example code.
Thank you!

@zekageri
Copy link
Author

So in theory i could do something like this:

Modbus::ResultCode err;
bool resCallback(Modbus::ResultCode event, uint16_t, void*) {
    err = event;
}
Modbus::ResultCode readSync(uint16_t Address, uint16_t start, uint16_t num, uint16_t* buf) {
  xSemaphoreTake(xMutex, portMAX_DELAY);
  if (mb.slave()){return Modbus::EX_GENERAL_FAILURE;}
  mb.readHreg(Address, start, buf, num, resCallback);
  while (mb.slave()) {vTaskDelay(1);mb.task();}
  Modbus::ResultCode res = err;
  xSemaphoreGive(xMutex);
  return res;
}

static const inline void Read_Exp_Inputs(){
    for(int i = 1; i < MAX_EXPANDER;i++){
        if(Expander_Info[i] != 0){
            uint16_t inputBuff[2];
            if (readSync(Expander_Info[i]->Address,Input_Reg,1,inputBuff) == Modbus::EX_SUCCESS) {
                uint16_t PIN = Detect_Bits_Change(Expander_Info[i]->Inputs[0],inputBuff[0]);
                Expander_Info[i]->Inputs[0]   = inputBuff[0];
            }
        }
    }
}

void ModBus_Reader_Task( void * parameter ){
       ModBus_Setup();
       vTaskDelay(100);
       Check_Bus();
     for ever{
       Read_Exp_Inputs();
     }
}

BTW: How do you color the the text between code tags? :D

@emelianov
Copy link
Owner

So in theory i could do something like this:

I think so.

BTW: How do you color the the text between code tags? :D

'```c'

@emelianov emelianov reopened this Dec 15, 2020
@emelianov
Copy link
Owner

emelianov commented Dec 16, 2020

On 14 Dec 2020, at 12:37, zekageri [email protected] wrote:

if (mb.slave()){return Modbus::EX_GENERAL_FAILURE;}
Serial.print("Reading: ");Serial.print(Address);Serial.print(" ");Serial.println(start); // THIS LINE IS NEVER PRINTED

Take a look new example

@zekageri
Copy link
Author

Thank you. Example is not found.
The thing is that i have to rewrite all my modbus read/write calls to this.
I have one big problem what i don't understand.

I have two continous loops in separate cores in separate tasks.
One is reading the inputs of my expander modules, and one is reading the button registers of some thermostats.
The expander addresses and the thermos addresses are far away from each other. ( expanders from 10 to 20, thermoses from 80 to 128 )

This is okay because the semaphore takes care of busy bus thing but on the thermos task i want to write to the thermoses sometimes. And if the semaphore is on the thermos task and the same tasks in an other function tries to take the semaphore and write to it it will not happen. Is it?

void Exp_Task(){
  while(1){
   readExpInputs();  // Gives semaphore
  }
 }
 
 void Therm_Task(){
  while(1){
   readThermButtons();                    // gets the semaphore or the someTimeWrite gets it? Idk?
   someTimeWrite_to_Therm();        //  write in every three sec for example. need to use semaphore because of modbus
  }
 }

@emelianov
Copy link
Owner

Two task read example doing virtually required thing.
You just need to create similar writeSync(..) function (by replacing readHreg with writeHreg). For sure all communication with Modbus have to be controlled by the mutex. Task that trying to access Modbus will stop till mutex release (if mutex is busy).

@zekageri
Copy link
Author

zekageri commented Jan 4, 2021

In the example you did this:

 if (mb.slave()){
    xSemaphoreGive(xMutex);
    return Modbus::EX_GENERAL_FAILURE;
  }

inside the readSync(); function. But if i want to do something like this in one Task :

#define MAX_EXPANDER_COUNT 20
static const inline void readContinous(){
    #define Input_Reg    0x0001
    uint16_t address = 10;
    uint16_t Input_Buffer[3];
    for(int i = 0; i < MAX_EXPANDER_COUNT;i++){
        if (readSync(address, Input_Reg, 2, Input_Buffer) == Modbus::EX_SUCCESS){
            Serial.println("Process the data in Input_Buffer");
        }
        address++;
    }
}

It will let one address out if mb.slave()

@emelianov
Copy link
Owner

Normally if you always access mb Modbus object using readSync() (or locking xMutex) return Modbus::EX_GENERAL_FAILURE; file never happen.

@zekageri
Copy link
Author

zekageri commented Jan 4, 2021

Thank you. I'm currently thinking to integrate this to the whole project. I have several mistakes in the code regarding modbus communication. I have to fill these holes in first. I will respond back with the tests as soon as i can! I really appreciate your help.

@zekageri
Copy link
Author

zekageri commented Jan 5, 2021

Wen the esp boots up, it will check the bus for "expanders". The idea is that it would write or read something for each expander on the bus, and if it is responding back with success, it would save its address. I did the following for this:

#define MAX_EXPANDER 10

typedef struct {
    uint16_t Inputs[1]      = {0};
    uint16_t OutPuts[1]   = {0};
    uint16_t Address       = 0;
} Expander_Struct;
Expander_Struct * Expander_Info[MAX_EXPANDER];

static const inline void checkExpanders(){
    uint16_t address = 0x0A;
    for(int i = 1; i < MAX_EXPANDER+1;i++){
        Serial.print("Checking address: ");Serial.print(address);
        uint16_t inputBuffer[1] = {0};
        if (writeSync(address,Output_Reg,1,inputBuffer) == Modbus::EX_SUCCESS){
            if(Expander_Info[i] == 0){Expander_Info[i] = new Expander_Struct;}
            Expander_Info[i]->Address = address;
            Serial.print(" - responded");
        }else{
            Serial.println(" - no response");
        }
        address++;
    }
    Serial.println("\nAll found expanders: ");
    for(int i = 0; i < MAX_EXPANDER;i++){
        if(Expander_Info[i] != 0){
            Serial.print("Address: ");
            Serial.println(Expander_Info[i]->Address);
        }
    }
}

This function is called on setup. I used your example for the writeSync function. If i write this buffer at startup: uint16_t inputBuffer[2] = {0x55,0x55}; and not this: uint16_t inputBuffer[1] = {0}; the expanders will successfully turn on their pins but i get this on the serial:

Checking address: 10 - no response
Checking address: 11 - no response
Checking address: 12 - no response
Checking address: 13 - no response
Checking address: 14 - no response
Checking address: 15 - no response
Checking address: 16 - no response
Checking address: 17 - no response
Checking address: 18 - no response
Checking address: 19 - no response

All found expanders:
Address: 1
Address: 12320

So this line: if (writeSync(address,Output_Reg,1,inputBuffer) == Modbus::EX_SUCCESS){} always evaulates to false for some reason even tho it was successfull

Using the example like this:

/************************************** TEST INPUT READ **************************************/
Modbus::ResultCode err;
bool resCallback(Modbus::ResultCode event, uint16_t, void*) {err = event;}

Modbus::ResultCode readSync(uint16_t Address, uint16_t start, uint16_t num, uint16_t* buf) {
  xSemaphoreTake(xMutex, portMAX_DELAY);
  if (mb.slave()){xSemaphoreGive(xMutex);return Modbus::EX_GENERAL_FAILURE;}
  //Serial.printf("SlaveID: %d Hreg %d\n", Address, start);
  mb.readHreg(Address, start, buf, num, resCallback);
  while (mb.slave()) {vTaskDelay(1);mb.task();}
  Modbus::ResultCode res = err;
  xSemaphoreGive(xMutex);
  return res;
}
/************************************** TEST INPUT READ **************************************/

/************************************** TEST INPUT WRITE **************************************/
Modbus::ResultCode writeSync(uint16_t Address, uint16_t start, uint16_t num, uint16_t* buf) {
  xSemaphoreTake(xMutex, portMAX_DELAY);
  if (mb.slave()){xSemaphoreGive(xMutex);return Modbus::EX_GENERAL_FAILURE;}
  //Serial.printf("SlaveID: %d Hreg %d\n", Address, start);
  mb.writeHreg(Address, start, buf, num, resCallback);
  while (mb.slave()) {vTaskDelay(1);mb.task();}
  Modbus::ResultCode res = err;
  xSemaphoreGive(xMutex);
  return res;
}
/************************************** TEST INPUT WRITE **************************************/

@zekageri
Copy link
Author

zekageri commented Jan 5, 2021

Okay, if i write anything with this readSync or writeSync i get Error E4, wich is a timeout. Does not matter if i increase the default timeout, i always get this error but the modules do what the esp told them to do.

@zekageri
Copy link
Author

zekageri commented Jan 5, 2021

Can i get the raw response with this somehow?

@zekageri
Copy link
Author

zekageri commented Jan 6, 2021

Okay the problem is that the expander module is responding back much faster than the esp can recognize it. The response comes less than 1ms. How can i avoid that?

@zekageri
Copy link
Author

zekageri commented Jan 6, 2021

I tried to debug it with a windows software called Multiway. On the software i can see the response from the slave module. It is responding back nicely. The esp gets a 228 error for some reason wich is E4 timeout. On an oscilloscope i can see that the response coming back less than 1ms.
Maybe the _txPin is not pulled down immidiatelly and the esp does not get the package??

@emelianov
Copy link
Owner

  1. According to Modbus specification recommended inter frame delay is at least 1.75 mS.
  2. _txPin is pulled immediate after return from Serial.flush() method. See no way to speedup this (except going deep to native RTOS implementation).
  3. Take a look related issue Timeout on ModbusRTU with Esp32S2 #85

@zekageri
Copy link
Author

zekageri commented Jan 6, 2021

Thank you for the response! I will read it carefully.

@zekageri
Copy link
Author

zekageri commented Jan 7, 2021

Thank you for all your help. Everything seems perfect. The syncron example is really good and straightforward. The last problem was at my hardware level. I managed to kill my rs485 chip. 👍

Currently this syncron example is the easyest and cleanest way for me!
Thanks again.

@zekageri zekageri closed this as completed Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants