bertmelis / esp32ModbusRTU

modbus RTU client for ESP32
MIT License
70 stars 44 forks source link

Is there any Error Checks for the busy bus? #25

Closed zekageri closed 4 years ago

zekageri commented 4 years ago

I want to know if there is a queue when someone using the bus. Does the request got dropped out or the modules have waiting for the bus to be available again?

Edit: When the master wants to write.

bertmelis commented 4 years ago

Hmmm, I don't think there is.

Got to implement this.

zekageri commented 4 years ago

So i tought that i will use a simpe boolean for that purpose. That didn't worked out well.

I have multiple slaves, and i don't know how many. At start up i'am trying to count these slaves, wich is working fine. I'am pushing one slave address into an array. I loop trought 128 slaves ( thats the max slaves i can have ) and if one is already in the array ( because its happens somehow ) i don't push it. So if the array filled, then i delete this task and creating another where i continously communicating with all the slaves and get the id's from the previously filled array.

I'am doing it like this:

I have this task on core 0 wich is running for ever because i have to continously check if there is some modification on one of my slaves pins.

void ModBus_Loop( void * parameter ){
  for ever{
    ModBus_RUN();
    delay(1);
  }
}

In the ModBus_RUN(); function i have this function:

boolean Bus_is_Busy = false;
int Reading_Address = 0;
static const inline void ModBus_Read_Existing(){
  if(!Bus_is_Busy){
    Bus_is_Busy = true;
    modbus.readHoldingRegisters(Address_Array[Reading_Address], Start_In_Reg_Address, Reg_Count);
    Reading_Address++;
    if(Address_Array[Reading_Address] == 0){Reading_Address = 0;}
  }
}

As you can see there is a boolean variable where i tell the whole program that the rs485 line is currently busy. I did this because the readings was not reliable. They were fluctuating continously. And were getting an error every f.. time.

And i have an other function wich is running every 2 second.

long thermosreadtime = millis();
static const inline void Write_Thermos(){
  if(millis() - thermosreadtime >= 2000 && !Bus_is_Busy){
    thermosreadtime = millis();
    Bus_is_Busy = true;
    modbus.writeMultHoldingRegisters(128,0,8,Thermos_Data);
    Thermos_Data[11]++; if(Thermos_Data[11] >= 60){Thermos_Data[11] = 1;}
    Thermos_Data[15]++; if(Thermos_Data[15] >= 8){Thermos_Data[15] = 1;}
  }
}

And if i get a data or an error, inside the handlers i clearing the boolean to let any other reading run.

modbus.onData([](uint8_t slaveAddress, esp32Modbus::FunctionCode fc, uint16_t address, uint8_t* data, size_t length) {
    if(slaveAddress >= 128){       // Thermos
    }else if(slaveAddress < 128){  // Expanders
      if(fc == 3){
        if(Can_Check_Array){
          String Info_Data = "";
          for (int i = 0; i < length; i++)
          {if(!(i <= 2)){if(!(i%2 == 0)){Info_Data += Get_ASCII(data[i]);}}}
          Check_Existing_Modules(slaveAddress,Info_Data);
        }
      }
    }
    Bus_is_Busy = false;
  });
  modbus.onError([](esp32Modbus::Error error, uint8_t slaveAddress) {
    if(Can_Check_Array){Check_Loop_Count();}
    Handle_Error(String(error,HEX),String(slaveAddress,HEX));
    Bus_is_Busy = false;
  });

Like this, the program usually stops at one of the slaves, continously reading just one, and sometimes but just rarely, asking an other one. I espected it to read all the slaves wich are in the array one by one.

But thats not the case. :O

Btw: i have a maximum of 15millis response time, therefor the defined timeout is 15millis and the queue size is 10. But none of that matters. I varied these and nothing changed.

Edit: If i reading just one slave, there is no error at all. O_o

bertmelis commented 4 years ago

I've looked into the code just now.

So what happens is this:

  1. you make a request
  2. the request is put into a queue (_addToQueue)
  3. a separate task checks for requests in the queue (_handleConnection)
  4. if there is one, it sends the request (from _handleConnection, call _send)
  5. task waits for response or timeout, callbacks are fired
  6. tasks loops back to "check for pending requests"

So actually I would think we're not sending on a busy bus.

zekageri commented 4 years ago

So in theory, i can't send if the bus is busy, instead it gets added to the queue and it sends later.

Interesting because now, i put a vTaskDelay(20); inbetween requests and it is working fine now. There is the queue, and the timeout and my boolean Bus_is_Busy, and even with the given conditions, i had to put a delay inbetween requests. If i lower the delay to 15 or 10 millis, the errors are coming back. Slower and fewer but they are there.

How is it possible? :D

bertmelis commented 4 years ago

I always gets added to the queue. Because sending and receiving is in another task and the queue does the syncing between the tasks.

Where do you add the delay? In your code?

You might want to raise the queue size: https://github.com/bertmelis/esp32ModbusRTU/blob/72dd86c0a0fa020c73ca4bf4b73653997a280f20/src/esp32ModbusRTU.h#L31 The queue is by default only 20 requests. If you loop over 128 slaves, the queue fills up fast...

zekageri commented 4 years ago

I have this function:

static const inline void ModBus_RUN(){
  if(Flip_Read){
    ModBus_Read_Existing();
    vTaskDelay(20);
    Write_Thermos();
  }
}

This has two functions in it. One for communicating the existing slaves and one for communicating a thermostat.

Address_Array contains the existing addresses of the slaves on the bus. This function is looping trought of all the slaves. One by one. After one request, 20 millis delay and check the thermostat wich is running in every second.

int Reading_Address = 0;
static const inline void ModBus_Read_Existing(){
  if(!Bus_is_Busy){
    Bus_is_Busy = true;
    modbus.readHoldingRegisters(Address_Array[Reading_Address], Start_In_Reg_Address, Reg_Count);
    Reading_Address++;
    if(Address_Array[Reading_Address] == 0){Reading_Address = 0;}
  }
}
long thermosreadtime = millis();
static const inline void Write_Thermos(){
  if(millis() - thermosreadtime >= 2000 && !Bus_is_Busy){
    thermosreadtime = millis();
    Bus_is_Busy = true;
    modbus.writeMultHoldingRegisters(128,0,8,Thermos_Data);
    Thermos_Data[11]++; if(Thermos_Data[11] >= 60){Thermos_Data[11] = 1;}
    Thermos_Data[15]++; if(Thermos_Data[15] >= 8){Thermos_Data[15] = 1;}
  }
}

The queue is not getting 128 requests just once at start up. After that the program communicates with the existing slaves on the bus, wich is currently 2.

bertmelis commented 4 years ago

Are you getting the response before you make the next request?

zekageri commented 4 years ago

It doesn't matter if i get data or an error, in both situations, i make the Bus_is_Busy bit to false and the request goes again. Or why do you ask? I don't call the modbus read if the previous is not finished. If is that what you asking.