Ppd42 x #577
Ppd42 x #577
Conversation
| @@ -13,7 +13,7 @@ lib_deps = | |||
| ESP Async [email protected] | |||
| [email protected] | |||
| [email protected] | |||
| ESPAsyncTCP@1.2.0 | |||
| ESPAsyncTCP@1.1.3 | |||
src/esphome/sensor/ppd42x_sensor.h
Outdated
|
|
||
| namespace sensor { | ||
|
|
||
| class Ppd42xSensorComponent : public PollingSensorComponent { |
There was a problem hiding this comment.
This class shouldn't inherit from PollingSensorComponent but from PollingComponent. In your current design both PM10 and pm2.5 values will be published to the same sensor, which is wrong. Please see PMSx003 example for how to split the two values.
src/esphome/sensor/ppd42x_sensor.cpp
Outdated
| std::string Ppd42xSensorComponent::icon() { return "mdi:arrow-expand-vertical"; } | ||
|
|
||
| int8_t Ppd42xSensorComponent::accuracy_decimals() { | ||
| return 2; // cm precision |
src/esphome/sensor/ppd42x_sensor.cpp
Outdated
|
|
||
| float Ppd42xSensorComponent::get_setup_priority() const { return setup_priority::HARDWARE_LATE; } | ||
| std::string Ppd42xSensorComponent::unit_of_measurement() { return "µg/m³"; } | ||
| std::string Ppd42xSensorComponent::icon() { return "mdi:arrow-expand-vertical"; } |
There was a problem hiding this comment.
See other particulate matter sensors, for example pmsx003
src/esphome/sensor/ppd42x_sensor.cpp
Outdated
| float result = Ppd42xSensorComponent::us_to_pm(this->timeout_us_, time_pm_10_0); | ||
| this->publish_state(result); | ||
| ESP_LOGD(TAG, "'%s' - Got PM10.0 Concentration: %.1f µg/m³", this->name_.c_str(), result); | ||
| this->publish_state(result); |
|
Your PR contains commits that are not part of this PR (see files tabs, changes are listed there that have nothing to do with this PR). This is due to an incorrect merge. Please see contributing guide for merging dev correctly. |
src/esphome/sensor/ppd42x.cpp
Outdated
| this->timeout_us_); | ||
| uint32_t duration_pl_10_0 = pulseIn(this->pl_10_0_sensor_->pl_pin_->get_pin(), | ||
| uint8_t(!this->pl_10_0_sensor_->pl_pin_->is_inverted()), | ||
| this->timeout_us_); |
There was a problem hiding this comment.
Don't do this inside loop() - Since the timeout is ms this will slow down the entire esphome core loop. That's ok when it's only executed every so often, but not if it's done every loop cycle.
There was a problem hiding this comment.
got it, i will protect by update cycle
There was a problem hiding this comment.
I would prefere to reuse what you did on pulse_count module with interruption, what is your advise?
src/esphome/sensor/ppd42x.cpp
Outdated
| this->lowpulseoccupancy_02_5_ = this->lowpulseoccupancy_02_5_ + duration_pl_02_5; | ||
| this->lowpulseoccupancy_10_0_ = this->lowpulseoccupancy_10_0_ + duration_pl_10_0; | ||
|
|
||
| if (now - this->starttime_ > this->timeout_us_) { |
There was a problem hiding this comment.
You're comparing a value in ms to one in us. Please just update and publish data in update
src/esphome/sensor/ppd42x.cpp
Outdated
| case PPD42X_TYPE: { | ||
| float pl_02_5_concentration = us_to_pl(this->lowpulseoccupancy_02_5_, this->timeout_us_); | ||
| float pl_10_0_concentration = us_to_pl(this->lowpulseoccupancy_10_0_, this->timeout_us_); | ||
| ESP_LOGD(TAG, "Got PM2.5 Concentration %f pcs/L, PM10.0 Concentration: %f pcs/L", pl_02_5_concentration, |
There was a problem hiding this comment.
| ESP_LOGD(TAG, "Got PM2.5 Concentration %f pcs/L, PM10.0 Concentration: %f pcs/L", pl_02_5_concentration, | |
| ESP_LOGD(TAG, "Got PM2.5 Concentration %.0f pcs/L, PM10.0 Concentration: %.0f pcs/L", pl_02_5_concentration, |
| this->pl_02_5_sensor_->publish_state(pl_02_5_concentration); | ||
| if (this->pl_10_0_sensor_ != nullptr) | ||
| this->pl_10_0_sensor_->publish_state(pl_10_0_concentration); | ||
| break; |
There was a problem hiding this comment.
What is the difference between these switch cases? They all look the same to me.
There was a problem hiding this comment.
we may have only one sensor ? PM2.5 or PM10.0 or both
There was a problem hiding this comment.
No, I mean why do you differentiate between this->ctype_ ? All the switch cases look the same to me.
| } | ||
| } | ||
| float PPD42XComponent::us_to_pl(uint32_t sample_length, uint32_t time_pm) { | ||
| float ratio = time_pm / (sample_length * 10.0f); |
There was a problem hiding this comment.
Why does this depend on the timeout value?
There was a problem hiding this comment.
this is the recommended max value to listen to !
There was a problem hiding this comment.
I mean why does this calculation depend on the timeout? The sensor doesn't know which timeout we chose, so how can this calculation depend on the timeout value this->timeout_us_ we chose?
There was a problem hiding this comment.
nop, the sensor send continuously data.
we cannot stay stuck too long to lisning the end of the signal, so we timeout...
src/esphome/sensor/ppd42x.h
Outdated
| uint32_t lowpulseoccupancy_10_0_{0}; | ||
|
|
||
| uint32_t last_transmission_{0}; | ||
| uint32_t ui_{60000}; |
There was a problem hiding this comment.
ui_ is the update interval, name too short ?
|
Hi Otto, can you please help to resolve this conflict? i do not know how to do it |
|
Any chance this gets merged soon? |
|
I'm also waiting for this merge. pin:
number: GPIO13 # your input
inverted: true |
Description:
Related issue (if applicable): fixes
Pull request in esphome with python changes (if applicable): esphome/esphome#
Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#
Checklist:
If user exposed functionality or configuration variables are added/changed: