Suggestion on how to read from two Slaves at the same time
maxdd opened this issue · 5 comments
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?
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.
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?
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.
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.
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.