Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MH-Z19 sensor has disabled auto-calibrating #1580

Open
eschava opened this issue Feb 25, 2019 · 7 comments
Open

MH-Z19 sensor has disabled auto-calibrating #1580

eschava opened this issue Feb 25, 2019 · 7 comments
Labels
enhancement New feature or request sensors

Comments

@eschava
Copy link
Contributor

eschava commented Feb 25, 2019

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

@eschava eschava added the enhancement New feature or request label Feb 25, 2019
@mcspr mcspr added the sensors label Feb 27, 2019
eschava added a commit to eschava/espurna that referenced this issue Feb 28, 2019
@mcspr
Copy link
Collaborator

mcspr commented Feb 28, 2019

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
letscontrolit/ESPEasy@a643596#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 added a commit to eschava/espurna that referenced this issue Feb 28, 2019
@eschava
Copy link
Contributor Author

eschava commented Mar 1, 2019

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
Copy link
Collaborator

mcspr commented Mar 1, 2019

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. letscontrolit/ESPEasy#466 (comment), letscontrolit/ESPEasy#466 (comment) 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
Copy link
Contributor Author

eschava commented Mar 1, 2019

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
Copy link
Collaborator

mcspr commented Mar 1, 2019

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:

  • check inside mhz::_read() if 0x3a 0x98 right before checksum byte are not set and send calibration command once they are gone (i.e. repeat the approach from espeasy)
    or
  • add new method to set _sensors_ready=false (see sensor.ino loop function) so espurna calls begin() again

edit to clarify, sry

eschava added a commit to eschava/espurna that referenced this issue Mar 1, 2019
@eschava
Copy link
Contributor Author

eschava commented Mar 1, 2019

OK, pushed new version

@mcspr
Copy link
Collaborator

mcspr commented Mar 1, 2019

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.

eschava added a commit to eschava/espurna that referenced this issue Mar 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request sensors
Projects
None yet
Development

No branches or pull requests

2 participants