yaacov / ArduinoModbusSlave

Modbus slave library for Arduino
ISC License
204 stars 74 forks source link

Strange behiavor on slave using write single register #72

Open rtu-dataframe opened 3 years ago

rtu-dataframe commented 3 years ago

Hi,

I'm using this library on arduino in order to make it as a Modbus Slave, the slave consists in a ATMEGA328P with a bunch of neopixel leds (8) attached, periodically - every 250ms - , the master (Library: ModbusMaster) will change the led colors, generating some random colors.

The speed is: 57600-8E1

I was experiencing very often a slave timeout on the master (226 error), even if the master timeout limit is not reached (2000ms).

After some experiments, i've found that if i add to the slave a small delay(10) at the end of the write register function, the problem almost disappears (the timeout errors will appear very few times, like one every 100 writes)

This is weird, i mean: is this a normal behavior? i need to slow down the response of my slave in order to get it work?

immagine

yaacov commented 3 years ago

hi, thanks for the question, and the how to fix :+1:

I'm adding a "help wanted label" in case someone know the answer and can help, my guess (without checking) is that you need a delay between data frames [1] and this delay makes the data frames valid.

[1] https://web-plc.com/blog/2017/06/01/rtu-modbus/

falahati commented 3 years ago

Can you share the code in the main?

rtu-dataframe commented 3 years ago

@falahati, here you go, as you can see it's pretty straightforward:


//tse.slave_buff is in external header, it's defined as following: uint8_t slave_buff[56];

uint8_t write_led_values(uint8_t fc, uint16_t address, uint16_t length)
{
    uint16_t led_value = slave.readRegisterFromBuffer(0);

    tse.slave_buff[address] = (uint8_t)led_value;
    board_leds[address] = CHSV(tse.slave_buff[address], 255,255);
    FastLED.show();

    delay(10); //here is the magic line!
    return STATUS_OK;
}

uint8_t read_values(uint8_t fc, uint16_t address, uint16_t length)
{

    for (int i = 0; i < length; i++) {
        // Write the state of the analog pin to the response buffer.
        slave.writeRegisterToBuffer(i, tse.slave_buff[address + i]);
    }

    return STATUS_OK;
}

void setup()
{
    FastLED.addLeds<NEOPIXEL, DATA_PIN>(board_leds, NUM_LEDS);

    pinMode(ctrl_led, OUTPUT);
    Serial.begin(57600, SERIAL_8E1);
    slave.begin(57600);

    board_leds[0] = CHSV(tse.slave_buff[0], 255, 255);
    board_leds[1] = CHSV(tse.slave_buff[1], 255, 255);
    board_leds[2] = CHSV(tse.slave_buff[2], 255, 255);
    board_leds[3] = CHSV(tse.slave_buff[3], 255, 255);
    board_leds[4] = CHSV(tse.slave_buff[4], 255, 255);
    board_leds[5] = CHSV(tse.slave_buff[5], 255, 255);
    board_leds[6] = CHSV(tse.slave_buff[6], 255, 255);
    board_leds[7] = CHSV(tse.slave_buff[7], 255, 255);
    FastLED.show();

    slave.cbVector[CB_WRITE_HOLDING_REGISTERS] = write_led_values;
    slave.cbVector[CB_READ_HOLDING_REGISTERS] = read_values;
}

unsigned long last_measurement = 0;
void loop()
{

    if ((millis() - last_measurement) > 1000)
    {
        digitalWrite(ctrl_led, HIGH); //control led, pretty useless but used to see when the loop goes here
        last_measurement = millis();
        digitalWrite(ctrl_led, LOW);
    } else if (millis() < last_measurement) {
        last_measurement = millis();
    }

    slave.poll();
}
falahati commented 3 years ago

Everything seems ok; the only thing that I can think of is that the master can not keep up with the speed of the slave. Try lowering the baudrate and see if it makes it more stable. Adding a delay to the write function only makes it slower to respond, and if that somehow helps, it means that either we are sending the response too fast or the master can't parse the message fast enough. As per Modbus specification, we consider a message as completed after 3.5T (1,750us) of silence for baudrate of 56700, and ready to be answered. It is possible that the master expects more time between messages. If this is the case, it indicates a bad design on the receiving library. Baudrate of 19200 might solve this if this is the case.

falahati commented 3 years ago

I don't see any time limitation or transfer time calculation on the codebase of the master's library. Maybe the library used for serial communication is overwriting the output buffer or maybe the hardware used to communicate with the bus is somehow incapable of handling a fast burst of data. Are you using hardware serial on both sides or you have a virtual serial library in place? Meanwhile, decreasing the baudrate should probably help.

rtu-dataframe commented 3 years ago

Thanks @falahati, i'm using serial hardware on both side: the Master side is a ESP32 and the slave side an arduino pro mini with ATMEGA328p running at 16Mhz.

What looks weird is that lowering the baudrate the error rate increases a lot!

balrok commented 3 years ago

Not an expert with modbus, but I'm currently reviewing multiple libraries and found this: https://github.com/angeloc/simplemodbusng/blob/master/SimpleModbusSlave/SimpleModbusSlave.cpp#L202 The comment also confirms @Simonefardella observation that a lower baudrate requires the delay even more.

rtu-dataframe commented 3 years ago

What are you experiencing with this library? it is ok? works without any issue or you need to add the additional delay?

falahati commented 3 years ago

Not an expert with modbus, but I'm currently reviewing multiple libraries and found this: https://github.com/angeloc/simplemodbusng/blob/master/SimpleModbusSlave/SimpleModbusSlave.cpp#L202 The comment also confirms @Simonefardella observation that a lower baudrate requires the delay even more.

Thanks for pointing us in the right direction. Apparently libraries out there are not as standard as I liked them to be. In any case, I never had a problem with the code and I couldn't find anything suspicious in the code of the rtu-modbud so I am not yet sure how this could happen.

However, if this actually works, the fix should be easy to implement and increase the delay for communication.