yaacov / ArduinoModbusSlave

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

ModBusSlave delivers bytes instead of words on FC03 How to fix ? #102

Open kurt-klingbeil opened 1 year ago

kurt-klingbeil commented 1 year ago

I have ModbusSlave installed and running, but instead of delivering words from the buffer as words it delivers bytes from the buffer as words

I just took code fragments from the examples and integrated into the application. I fiddled around with changing various types to 16 from 8 but nothing resolved rf95rk_reldg_RTD_server_Mega_10_IO_MB_FL_12.zip

Any suggestions, clues, or outright fixes most welcome. I have tried to switch to other modbus libraries, but a few failed to even compile - getting stuck in the library code

yaacov commented 1 year ago

Hi, thanks for the issue, i added a help wanted label in case someone can help.

device111 commented 1 year ago

Hello, first thing, delete this:

//remove from final version if (Serial1.available() > 100) { byte incomingByte1 = Serial1.read(); // read the incoming byte: Serial.print(">>"); Serial.println(incomingByte1);

  digitalWrite(RS485DEPin, HIGH);        //  (Pin RS485DEPin always LOW to receive value from Master)
  delay(10);

  Serial1.print(">>"); Serial1.println(incomingByte1);
  digitalWrite(RS485DEPin, LOW);        //  (Pin RS485DEPin always LOW to receive value from Master)
  delay(10);

}

You should not read the Serial1. the modbus library do this for you. If you read the Serial1, the bytes are removed from the Serial1 buffer.

  1. Dont't use "delays" in your code.

Here you read only bytes. (uint8_t):

// Handle the function code Read Holding Registers (FC=03) and write back the values from the EEPROM (holding registers). uint8_t readMemory(uint8_t fc, uint16_t address, uint16_t length) { // Read the requested ClientStatus registers. for (int i = 0; i < length; i++) { // Below 50 is reserved for pinModes, above 50 is free to use. if (address >= 0) { uint8_t value; value = hr.MB_HoldReg[i+address];

        slave.writeRegisterToBuffer(i, value);
    }

}

// Serial.print("FC "); Serial.print(fc); Serial.print(" Address "); Serial.print(address); // Serial.println(" OK");

return STATUS_OK;

}

check the things and try again. br

device111 commented 1 year ago

here is the next one: union { struct ClientStatusEx ClSt[N_Slots]; uint8_t MB_HoldReg[sizeof(ClientStatusEx) * N_Slots]; } hr;

declare it as uint16_t

  1. use Modbus wtith own array for registers.
  2. clean your code. It's verry hard to read.
kurt-klingbeil commented 1 year ago

thank you for reviewing my code I have assembled it by copy/pasting example code from several libraries I am used to having separate modules in separate files and have not quite figured out the stylistic aspects of putting all the code into one file...

My reading and experience of the //remove from final version code is that serial1 will only be read directly when the available character count reaches 100. A kind of debugging aid should the modbus processing stop. So far I have not experienced such loss of data.

I did previously experiment with changing the union { uint8_t MB_HoldReg[... to uint16_t however that conflicts with the RHReliableDatagram code

I will have to do as you suggest and create a separate array for the modbus data