xoseperez / espurna

Home automation firmware for ESP8266-based devices
http://tinkerman.cat
GNU General Public License v3.0
2.98k stars 635 forks source link

MH-Z19 sensor has disabled auto-calibrating #1580

Open eschava opened 5 years ago

eschava commented 5 years ago

MH-Z19 sensor with auto-calibrating disabled can show incorrect values when working for a long time I see that the library has the required methods (calibrateAuto/calibrateZero/calibrateSpan) but they are not used anywhere except setting autocalibrating to false during initialization

Ideally, there should be GUI setting disabling or enabling autocalibration and also command calibrating device to zero. But since this functionality is very custom, I think having settings parameter that could be adjusted in the text file and restored using admin page should be enough

mcspr commented 5 years ago

Regarding #1592 , it disables calibration and then re-enables it again. Maybe just wire things in the sensor::begin() + additional flag after read to send calibration setting? And because calibrateAuto() calls yield() + does serial reads / writes, I think it will straight up crash when saving settings in the web interface.

I was looking at the ESPEasy MHZ code, pdf and referenced articles: https://github.com/letscontrolit/ESPEasy/blob/65a32aad7529c98f3e102de4bac0e0fed02378df/src/_P049_MHZ19.ino#L502 https://github.com/letscontrolit/ESPEasy/commit/a643596c487a3ae62618b5bcfb834471a19e9dd0#diff-e54c11e443e7f6b94b726d36f9c2e952 and related issue https://revspace.nl/MHZ19#Command_0x86_.28read_concentration.29

Plugin waits for sensor to settle before disabling the calibration. And, technically, there is no need to enable it on boot as it is already enabled.

eschava commented 5 years ago

I've added disabling of auto-calibration to the _sensorConfigure method because all other code related to calibration is present there.

And, technically, there is no need to enable it on boot as it is already enabled. You mean that auto-calibration is enabled by default so we do not need disable and enable it after?

mcspr commented 5 years ago

You mean that auto-calibration is enabled by default so we do not need disable and enable it after?

Looks like it. Datasheet claims that it is on by default on boot. https://github.com/letscontrolit/ESPEasy/issues/466#issuecomment-381442197, https://github.com/letscontrolit/ESPEasy/issues/466#issuecomment-381432013 even suggests sending this value every 24h, but idk why would sensor reset it's settings. From pdf for 1.0:

ABC logic function refers to that sensor itself do zero point judgment and automatic calibration procedure intelligently after a continuous operation period. The automatic calibration cycle is every 24 hours after powered on. The zero point of automatic calibration is 400ppm. From July 2015, the default setting is with built-in automatic calibration function if no special request.

And based on the links above, code should probably check if sensor is booting up and apply calibration setting after that.

I've added disabling of auto-calibration to the _sensorConfigure method because all other code related to calibration is present there.

I see the reasoning, but the problem is that it calls yield(), which will panic system when called from system context (lwip callback aka websocket request handler). So either configuring needs to happen in loop() or calibration command needs to happen somewhere else.

eschava commented 5 years ago

What do you think then if we remove calibrateAuto(false); from MHZ19Sensor.begin() method and execute it from _sensorConfigure only if getSetting("mhz19CalibrateAuto", 0).toInt() != 1 ?

mcspr commented 5 years ago

It can stay in begin() and setting can be passed to the sensor via some setter method, like pins do. And force sensor to send calibration command when value changes. So maybe:

edit to clarify, sry

eschava commented 5 years ago

OK, pushed new version

mcspr commented 5 years ago

Added that. As for Web, ideally it would somehow notify the client about available setting instead of manually typing it out in the html, but that's not yet implemented.