tom1422 / ESPHome-HPMA115S0-Sensor-Component

Integration for the HPMA115S0 Dust and Particle Sensor into ESPHome
5 stars 2 forks source link

Updated the read function to work with the compact version of the sen… #4

Closed jeffeb3 closed 2 years ago

jeffeb3 commented 2 years ago

…sor (003/004).

TL;DR:

These code changes should work for the bigger version and the compact version of the sensor.

Story

I tried to use a similar format and style, but the if..else was confusing me. Sorry for inverting the logic on it.

The compact version of the sensor has a longer message:

40 0D 04 00
30 00 31 00 32
00 33 00 00 00
00 E9

It outputs PM1, PM4, in addition to PM2.5 and PM10. There is this useful note in the datasheet though:

PM1.0 in μg/m3, PM4.0 in μg/m3, and PM10 in μg/m3 are calculated from PM 2.5 readings. I'm not that interested in pushing in the two other values if they are just simulated from PM2.5, so I'm not worrying about that.

These changes I've made read the LEN and then read in that many bytes from the serial port into messageBuffer. Then I added a checksum function to compute the checksum on a buffer.

Verification

I don't have the non-compact version, but I can verify that before I made these changes the code was complaining as expected:

[16:40:06][E][hpma115s0.sensor:042]: Read Values Failed - See Previous Message 
[16:40:07][D][uart_debug:114]: <<< 40:0D:04:00:02:00:03:00
[16:40:11][D][uart_debug:114]: <<< 03:00:03:00:00:00:00:A4
[16:40:11][D][uart_debug:114]: >>> 68:01:04:93
[16:40:11][E][hpma115s0.sensor:097]: Invalid Header - Check debug data if this happens again
[16:40:11][E][hpma115s0.sensor:098]: HEAD 64 LEN 13 COMD 4 DF1 0 DF2 2 DF3 0 DF4 3 CS 0

And after these changes, it works as expected:

[17:47:32][D][uart_debug:114]: >>> 68:01:04:93
[17:47:32][D][sensor:127]: 'Particulate Matter <2.5µm Concentration': Sending state 2.00000 <2.5µm with 0 decimals of accuracy
[17:47:32][D][sensor:127]: 'Particulate Matter <10.0µm Concentration': Sending state 2.00000 <10.0µm with 0 decimals of accuracy
[17:47:32][D][uart_debug:114]: <<< 40:0D:04:00:01:00:02:00:02:00:02:00:00:00:00:A8

(These logs have a debug: on the uart).

Side Note

I have been using my own implementation on these for a long time, and I finally had to update them and I am moving from my home grown solution to esphome. I also committed the code I have been using for all that time:

https://github.com/jeffeb3/airq/blob/master/src/HMPA.cpp

I also published AQI values (which are from the EPA, in the US). I'm not sure if that belongs in this code, or in the esphome config file:

https://github.com/jeffeb3/airq/blob/master/src/HMPA.cpp#L58

If that seems useful, I can help add it in. If not, then maybe we can just add it to the documentation to help people compute those, to compare with the AQI published outside.

tom1422 commented 2 years ago

Just looked through the code, looks good. The way you have done the if statements is much more readable! Log output from standard version:

[12:47:32][D][uart_debug:114]: >>> 68:01:04:93
[12:47:32][D][sensor:127]: 'Particulate Matter <2.5µm Concentration': Sending state 3.00000 <2.5µm with 0 decimals of accuracy
[12:47:32][D][sensor:127]: 'Particulate Matter <10.0µm Concentration': Sending state 4.00000 <10.0µm with 0 decimals of accuracy
[12:47:32][D][uart_debug:114]: <<< 40:05:04:00:03:00:04:B0

The compiler is complaining about the not statements so I added brackets (minor detail). As for the AQI, I would probably implement this in esphome or home assistant but other sensors implement it directly in code, which should be easy. Also, it should be possible to get the PM4 and PM1 values showing up on esphome with just a change of the config file from the user.

tom1422 commented 2 years ago

It works with the PM1 and PM4 values now if you specify it in the config file. The next step would be to implement the AQI reading. Looking at existing sensors they allow for multiple types of AQI reading (there are different ones for different regions) and they implement it using an enum. As your code appears to be the standard one it make sense just to use that for now.

jeffeb3 commented 2 years ago

Thanks for the attention and work on this. This will hopefully keep these sensors up to date in the future.

Do you plan on trying to get this merged into the esphome core repo? I have done that with the RadonEye sensor and it actually works pretty smoothly. You can just follow the templates and there is a bunch of automation to make the code go through clang-format and clang-tidy. The docs are a smooth process as well. After that, the esphome github pings me if someone opens an issue related to RadonEye (which I have no power to help with now, since I don't have that sensor anymore, long story).

You can probably see the benefits, but just to be explicit: 1) Users (myself included) will get any updates when they update esphome. 2) More people will see it (which can be good or bad, but hopefully that means more features and better quality). 3) Other people can use it as an example. I know I was grepping through the components when editing this code.