Suggestion on how to read from two Slaves at the same time

[maxdd]

Hello,
i have an inverter (fronius) and a smart meter.
The smart meter itself is read through the inverter with no problem.
Sometimes, though, i have a AsyncTCP kernel panic fault (i assume it is from Async lib)
My "read" function is the following

for (uint8_t i = 0; i < _numberfroniusRegisters; i++) {
        uint16_t packetId = _fronius.readHoldingRegisters(_froniusRegisters[i].address, _froniusRegisters[i].length);
        if (packetId > 0) {
            _froniusRegisters[i].packetId = packetId;
        } else {
            Serial.print("reading Primo error\n");
        }
    }
    delay(500); // test
for (uint8_t i = 0; i < _numbersmartMeterRegisters; i++) {
        uint16_t packetId = _smartMeter.readHoldingRegisters(_smartMeterRegisters[i].address, _smartMeterRegisters[i].length);
        if (packetId > 0) {
            _smartMeterRegisters[i].packetId = packetId;
        } else {
            Serial.print("reading Smart Meter error\n");
        }
    }

the onData() and onError() definition are inside the class init and are something like this

_fronius.onData([&](uint16_t packet, uint8_t slave, esp32Modbus::FunctionCode fc, uint8_t* data, uint16_t len) {
        for (uint8_t i = 0; i < _numberfroniusRegisters; ++i) {
            if (_froniusRegisters[i].packetId == packet) {
                _froniusRegisters[i].packetId = 0;
                if (_froniusRegisters[i].address == 40091) {
                    union u_tag {
                        uint32_t b;
                        float fval;
                    } u;
                    u.b = (data[0] << 24) | (data[1] << 16) | (data[2] << 8) | (data[3]);
                    _fronius_power = u.fval;
                    if (!isnan(_fronius_power)) {
                        _fronius_power = u.fval;
                        _produced_power = u.fval;
                        _average_fronius_power = (_average_fronius_power * idx + u.fval) / (idx + 1);
                        Serial.printf("_produced_power: %.2f\n", _produced_power);
                        Serial.printf("_average_fronius_power: %.2f\n", _average_fronius_power);
                    }
                }
            }
        }
    });

Do you believe that calling them sequentially like that can be the reason i have these crashes?
I will soon provide you an example of a crash

Moreover in the initialization list of my main class i have the following

 , _fronius(1, { 192, 168, 188, 62 }, 502)
 , _smartMeter(240, { 192, 168, 188, 62 }, 502)

is there a simpler way in order to avoid declaring two "esp32ModbusTCP" and only using one that can access two slave ids?

Can i extend the API of

uint16_t esp32ModbusTCP::readHoldingRegisters(uint16_t address, uint16_t numberRegisters) {
  esp32ModbusTCPInternals::ModbusRequest* request =
    new esp32ModbusTCPInternals::ModbusRequest03(_serverID, address, numberRegisters);
  return _addToQueue(request);
}

to take in also the different _serverID ? But how do i change the callback in order to understand which ID the information is from?

[bertmelis]

ModbusTCP is TCP based. It creates a separate connection to each slave. So yes, you'll need to have 2 class instances. (contrary to ModbusRTU where all slaves share the same bus).

Progress on this lib has stalled as I don't have any live modbus equipment anymore. I'm very busy the coming weeks but I can take a deeper look into the problem in 2 weeks or so.

[maxdd]

While i agree with what you have written, in my case the smart meter is RTU to the inverter which then converts the information on TCP with a different ID. This means that potentially only one TCP might be enough (to the inverter) as long as the payload of a given message carries the wanted ID. Am i wrong?

[maxdd]

I want to also add that maybe in the code below

ModbusRequest::ModbusRequest(size_t length) :
  ModbusMessage(nullptr, length),  // buffer will be set in constructor body
  _packetId(0),
  _slaveAddress(0),
  _functionCode(0),
  _address(0),
  _byteCount(0) {
    _buffer = new uint8_t[length];
    _packetId = ++_lastPacketId;
    if (_lastPacketId == 0) _lastPacketId = 1;
  }

we might have a race condition with the variabile _lastPacketId since it is a static class variable.
So in my case that might be a potential culprit.

[maxdd]

In order to have a single class/queue i've added the following method

uint16_t esp32ModbusTCP::readHoldingRegisters_Meter(uint16_t address, uint16_t numberRegisters) {
  esp32ModbusTCPInternals::ModbusRequest* request =
    new esp32ModbusTCPInternals::ModbusRequest03(_serverID_Meter, address, numberRegisters);
  return _addToQueue(request);
}

then i use something like

_fronius.onData([&](uint16_t packet, uint8_t slaveAddress, esp32Modbus::FunctionCode fc, uint8_t* data, uint16_t len) {
        if (slaveAddress == 1){
             //from fronius
        } else if (slaveAddress == 240){
            //from Meter
        }
}

where _serverID_Meter is passed in the constructor in the same way as _server_ID.
I'll test it during the weekend to see if it is more stable.

[bertmelis]

I see, your TCP meter acts as a modbus gateway. Maybe create an overloaded method with the extra argument?

The race condition is real, so has to be fixed.