andresarmento / modbus-arduino

A library that allows your Arduino to communicate via Modbus protocol, acting as a slave (master in development). Supports serial (RS-232, RS-485) and IP via Ethernet (Modbus IP).
BSD 3-Clause "New" or "Revised" License
453 stars 267 forks source link

write multiple holding registers #36

Closed boulirox closed 4 years ago

boulirox commented 6 years ago

Hello,

I found a bug with the write multiple holding registers function. The problem was that the variable was freed before the values were set in the registers. I'm not sure how to commit my fix so here's the code if someone want to do it.

void Modbus::writeMultipleRegisters(byte frame,word startreg, word numoutputs, byte bytecount) { //Check value if (numoutputs < 0x0001 || numoutputs > 0x007B || bytecount != 2 numoutputs) { this->exceptionResponse(MB_FC_WRITE_REGS, MB_EX_ILLEGAL_VALUE); return; }

//Check Address (startreg...startreg + numregs)
for (int k = 0; k < numoutputs; k++) {
    if (!this->searchRegister(startreg + 40001 + k)) {
        this->exceptionResponse(MB_FC_WRITE_REGS, MB_EX_ILLEGAL_ADDRESS);
        return;
    }
}

int nbOutputs = numoutputs;
word val;
word i = 0;
while(nbOutputs--) {
    val = val = (word)frame[6+i*2] << 8 | (word)frame[7+i*2];
    this->Hreg(startreg + i, val);
    i++;
}   

//Clean frame buffer
free(_frame);
_len = 5;
_frame = (byte *) malloc(_len);
if (!_frame) {
    this->exceptionResponse(MB_FC_WRITE_REGS, MB_EX_SLAVE_FAILURE);
    return;
}

_frame[0] = MB_FC_WRITE_REGS;
_frame[1] = startreg >> 8;
_frame[2] = startreg & 0x00FF;
_frame[3] = numoutputs >> 8;
_frame[4] = numoutputs & 0x00FF;

_reply = MB_REPLY_NORMAL;

}

dlabun commented 6 years ago

I've implemented your fix on my fork... Could you test my fork to see if the problem is fixed? https://github.com/dlabun/modbus-arduino

eboxmaker commented 4 years ago

我也发现这个问题了。解决办法如下 byte buffer; uint16_t size = ebox_get_sizeof_ptr(frame - 1); buffer = (byte )ebox_malloc(size); memcpy(buffer,frame,size); . . . uint16_t val; uint16_t i = 0; while(numoutputs--) { val = (((uint16_t)buffer[6 + i 2]) << 8 | (uint16_t)buffer[7 + i 2]); this->Hreg(startreg + i, val); i++; } ebox_free(buffer);

eboxmaker commented 4 years ago

I've implemented your fix on my fork... Could you test my fork to see if the problem is fixed? https://github.com/dlabun/modbus-arduino

你这个方发我实验了下。回复给主机的消息中出现错误信息