Buffer overflows

[rercek]

Hello,

FYI, this library has a systematic buffer overflow when you read a register with readReg. Indeed, the variable buf should have a length of 3 bytes and not only 2 bytes. Third byte is used for CRC8 in readReg function. For example

float DFRobot_MLX90614::getAmbientTempCelsius(void) {
 uint8_t buf[3]; //3 and not 2 !
 readReg(MLX90614_TA, buf);
 ...

Here is the corrected code with the following improvements:

• Fixed bug related to buffer overflows when reading registers.
• Write a new value to a register only if that value is not already set (read before write).
• New read (and/or write) functions for Emissivity, FIR, IIR, Gain and Raw Temperatures!

Best regards,

[qsjhyy]

You can verify the function readReg first, I have put the crc verification step in it, buf buffer only receives data, so there is no problem.

size_t DFRobot_MLX90614_I2C::readReg(uint8_t reg, void* pBuf)
{
  size_t count = 0;
  uint8_t pec = 0;
  if (NULL == pBuf) {
    DBG("pBuf ERROR!! : null pointer");
  }
  uint8_t* _pBuf = (uint8_t*)pBuf;

  _pWire->beginTransmission(_deviceAddr);
  _pWire->write(reg);
  if (0 != _pWire->endTransmission(false)) {
    // Used Wire.endTransmission() to end a slave transmission started by beginTransmission() and arranged by write().
    DBG("endTransmission ERROR!!");
  } else {
    // Master device requests size bytes from slave device, which can be accepted by master device with read() or available()
    _pWire->requestFrom(_deviceAddr, (uint8_t)3);

    while (_pWire->available()) {
      if (2 > count) {   // The incoming buf is only two bytes, and crossing the boundary will cause an error in the previous buf data
        _pBuf[count++] = _pWire->read();   // Use read() to receive and put into buf
      } else {
        pec = _pWire->read();
      }
      // DBG(_pBuf[count-1], HEX);
    }
    _pWire->endTransmission();

    // the array prepared for calculating the check code
    unsigned char crc_read[6] = { (uint8_t)(_deviceAddr << 1), reg, (uint8_t)((_deviceAddr << 1) | 1), _pBuf[0], _pBuf[1], '\0' };

    if (pec != crc8Polyomial107(crc_read, 5)) {
      count = 0;
      DBG("crc8Polyomial107 ERROR!!");
      DBG(pec, HEX);
    }
  }

  return count;
}

As for the optimization you said, if you are interested, you can carry out branch push, and you can merge after verification.

[rercek]

Dear qsjhyy,

Thank you very much for your reply.

The code you propose for readReg should work perfectly now but it is not (yet) the current code in the master branch of this repository, see here

For the optimization, I will create a new branch during this week on your repo since I'm little busy right now.

Kind regards,
Rudy

[qsjhyy]

I am sorry, it is indeed my negligence, after the code was corrected, I did not notice the upload failure. Now it has been reuploaded, thank you very much.

[rercek]

No sorry, it happens ;-)
By the way, thank you very much for developing this library with a code easy to understand and also easy to update/change ;-)

[rercek]

FYI, I forked your project on github since it is not allowed to create a new branch on your repo and included the new (read/get) functions in your current code on a dev branch. I've also modified a little your functions to update a register only if the current value has not yet been saved in the register (read before write).

The code still compiles well but, since I'm homeworking till the end of the week, I don't have access to a mlx sensor. So, I'll just have to check next week that everything still works as expected ;-)

Meanwhile, if you have time, don't hesitate to test it. Thank you.

[qsjhyy]

Maybe I was wrong before, I meant that branch push like this, after the test is correct, I will directly merge. Thanks again for your code optimization.

[rercek]

Hi,

Thank you for your reply.
I test it the new library with a MLX90614DCI and an ESP32 board (WROOM). There was still a small bug I fixed in your original code with the setMeasured. I didn't test it with other microcontrollers in your readme. I currently only work with esp32 (original version; wroom dual core) and nano. Hope, it will work with other microcontrolers.

By the way, I directly ask a pull resquest from my dev branch.

Kind regards,
Rudy

[qsjhyy]

Thank you very much, but you submitted a lot of content, I will conduct a detailed test when I am free, after the test is completed, I will merge the branch.

[rercek]

Yes, thank you, you can test it when you have time, sorry for the extra work.

Let us notice that I add functions to change the MLX gain but the temperatures values are wrong if you modify it. So, these (write) functions should not be used by default. Except, for example, if you add an optic to a MLX90614AAA (FoV 90°) to reduce its FoV, the temperatures values have to be corrected. In order to obtain more "precise" values, the gain should be increased from 12.5 (if I remember correctly) to 100 and the temperature has to computed using the temperature measurements and a calibration.