search

tobof / openhab-addons

The next-generation open Home Automation Bus (openHAB)
Eclipse Public License 2.0
39 stars 30 forks source link

Add CO2 sensor #44

Closed FutureCow closed 7 years ago

FutureCow commented 7 years ago

I don't know if i did this the right way. Just copy and paste a couple of things around. So it might be completely wrong!

tobof commented 7 years ago

Thank you for the PR!! :+1:

andreacioni commented 7 years ago

Hi @FutureCow, just a question about your PR. Why have you created a new channel named "co2-level"? We have already a "level" channel. I ask you that because I've done a big refactoring that will be merged soon with Tim work and this change is confusing me 😄. It will be better having only one channel for every MySensors variable because add new one every time we add a sensor could destroy re-usability. What do you think @tobof ?

UPDATE:This is also strange. If AIR_QUALITY presentation message arrives why it must be a CO2 sensor?

tobof commented 7 years ago

Hey @andreacioni ,

this was exactly my first thought too. Why use a new channel? I found an answer and because of that deleted my comment above. ;-)

image

In PaperUI Control the label makes a difference. In the item and sitemap configuration you may do what you want.

andreacioni commented 7 years ago

@tobof Hmmm ok, but do you think is enough to create a special channel for it? For example if someone want to measure methane we have to add a "methane-level-channel" and so on...

In my opinion the best thing to do is to remove "co2-level-channel" and in paper-ui leave: "Level" as label. On basic/classic/... ui everyone as the ability to customize label according to his preference.

I say that because if you see changes on DiscoveryService every new air quality sensor will be added as CO2 sensor (using the "co2-level-channel") and this is wrong.

FutureCow commented 7 years ago

@andreacioni I think you are right. It was a simple solution for a problem that doesn't exist. So better to leave the label: "Level" and changed it inside the sitemap etc. I think @tobof already changed CO2_LEVEL thing to AIR_QUALITY? https://github.com/tobof/openhab2-addons/commit/3d22695dc85ad5513be5a98da8811df9a7f2de4f