WifWaf / MH-Z19

For Arduino Boards (&ESP32). Additional Examples/Commands., Hardware/Software Serial
GNU Lesser General Public License v3.0
195 stars 39 forks source link

fixed for ESP8266 or in general fast controllers #11

Closed hixfield closed 4 years ago

hixfield commented 4 years ago

Fixes the issue reported on the wemos d1 mini. The problem is that on fast controllers the implementation did not wait until 9bytes of reply ware received on the serial connection before trying to decode them.

hixfield commented 4 years ago

fixes the issue #10

hixfield commented 4 years ago

@WifWaf I understand the idea to log all the flushed bytes. But if we do

inBytes[1] = mySerial->available();
        for(uint8_t x = 0; x < inBytes[1]; x++)
        {

we still run a higher risk that bytes will stay in the buffer why not move the available() into the loop like this? I don't understand why this would lockup when device is in error?

        while(mySerial->available() >0)
        {
          inBytes[0] = mySerial->read();
WifWaf commented 4 years ago

With available() > 0, if a device errors (e.g. steaming continually bytes to the bus), a while condition can never be broken and a controller can "lock-up".

Reading the buffer size first sets a finite cap (as the buffer size has a limit). So at worst you'll receive the clearing byte warnings, then CRC errors, but still have an opportunity to execute code elsewhere in a sketch. I guess the data waiting should be less than 9 too, so you could just read 9 bytes and break on read() returning -1?

However, it probably doesn't matter too much here... given it's reading just after a timeout on not receiving enough data .

hixfield commented 4 years ago

@WifWaf CR updated!

WifWaf commented 4 years ago

Great, thanks for fix @hixfield! :)

hixfield commented 4 years ago

@WifWaf I just fetched the lib on another computer but still got the old one (without this fix). Do we need to update any version or anything else to make it available to the community?

WifWaf commented 4 years ago

@hixfield I'm not sure git requests work (do you have to pull a development version?).

I've updated the release anyway, as there are quite a few changes since the last. It should pull the newest version.

hixfield commented 4 years ago

@WifWaf found info here https://github.com/arduino/Arduino/wiki/Library-Manager-FAQ if you updated the release and properties file all should be picked up automatically by the Arduino lib sync process running each hour.

WifWaf commented 4 years ago

@hixfield ahh got you., for some reason, I thought you were using the git command.

I've just tried compiling and there's an issue. So will fix this in a few minutes.

hixfield commented 4 years ago

@WifWaf By the way the temperature returned is way off. I get like 30C while is 23C. Its an undocument feature right of the sensor?

WifWaf commented 4 years ago

@hixfield I think it's included in one of the datasheets, perhaps not for this specific model (I write this library some time back).

However, my understanding is that the temperature value is only used for calibration purposes (part of its algorithm), so doesn't have much bearing on the "actual temperature".