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

[airgradient] Initial contribution #16584

Merged
merged 53 commits into from
May 9, 2024
Merged

[airgradient] Initial contribution #16584

merged 53 commits into from
May 9, 2024

Conversation

austvik
Copy link
Contributor

@austvik austvik commented Mar 27, 2024

New binding: Airgradient

This binding lets you get data from https://www.airgradient.com/ air quality sensors and use them for you home automation or dashboards.

AirGradient air quality sensors are open source and open hardware, so should be attractive for home automation enthusiasts.

This binding can either poll from data from the AirGradient Cloud API or communicate directly to sensors on the network.

The binding has been tested against the latest firmware with both outdoor O-1PST and indoor I-9PSL sensors.

It has a thread here: https://community.openhab.org/t/airgradient-api-binding/152798

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@austvik austvik requested a review from a team as a code owner March 27, 2024 14:32
@austvik
Copy link
Contributor Author

austvik commented Mar 27, 2024

(I am not able to set "new binding" label)

@jlaur jlaur added the new binding If someone has started to work on a binding. For a new binding PR. label Mar 27, 2024
@lolodomo
Copy link
Contributor

It does not build. I let you check the logs but I already see that a required file is missing in bom folder.

Thanks to lolodomo for the find!

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@austvik
Copy link
Contributor Author

austvik commented Mar 28, 2024

It does not build. I let you check the logs but I already see that a required file is missing in bom folder.

Thank you @lolodomo ! I had some issues with forgetting signoffs on some commits and forgot that change when I started from scratch.

Copy link
Member

@holgerfriedrich holgerfriedrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this is not a full review and I am not a maintainer.

I just double checked for SAT warnings (just a few, see below), checked that javadoc runs without errors (the warnings are fine), and quickly looked into the README.

Find my suggestions below.

austvik and others added 2 commits April 1, 2024 11:37
Thank you, @holgerfriedrich

Co-authored-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@austvik
Copy link
Contributor Author

austvik commented Apr 1, 2024

Thank you, @holgerfriedrich! I believe all your excellent suggestions are incorporated now.

- Required token (not required for local)
- Stack trace when emptying inbox

Make some more constants.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@lsiepel
Copy link
Contributor

lsiepel commented Apr 11, 2024

Thanks for your contribution!. As there are more new bindings waiting to be reviewed, and usually new bindings can take up some time to be reviewed and adjusted, i would like to ask you to do a selff-review before a maintainer picks this up.
A checklist is available here: https://github.com/openhab/openhab-addons/wiki/Review-Checklist

ftr: it is not mandatory, it is just an attempt to improve the experience/time spend on PR's. Let's see if this turns out to be a step in the good direction.

@austvik
Copy link
Contributor Author

austvik commented Apr 12, 2024

Thank you, @lsiepel ! I'm currently traveling, but will run that self check when I am back in about a week.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@austvik
Copy link
Contributor Author

austvik commented Apr 27, 2024

@lsiepel I have now gone through the review checklist and fixed the two issues I found. There is one point there I might still be sinning against: 32. Cache results of getConfigAs(). Reading it more frequently got rid of many non-null warnings. Do you have any guidance for how to deal with config null safe?

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
used to debug the sensors when troubleshooting.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for performing the self-review. Code is allready in a good shape, i just have some comments and questions.

austvik and others added 10 commits April 27, 2024 16:52
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/discovery/AirGradientLocationDiscoveryService.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/handler/AirGradientAPIHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/handler/AirGradientAPIHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/handler/AirGradientAPIHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/handler/AirGradientAPIHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/handler/AirGradientAPIHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/handler/AirGradientAPIHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@lsiepel
Copy link
Contributor

lsiepel commented May 7, 2024

kes sense to get this in in the

Can you give some insights of the size (lines changed) and the estimated time of publishing the PR?

@austvik
Copy link
Contributor Author

austvik commented May 8, 2024

The one with model property and separating out creating of states from the controller (for reuse):

austvik@austvik-desktop:~/src/openhab-addons$` git show 6d32039c9f111aeeec2466bf25ead30d3cb5f357 | diffstat
 main/java/org/openhab/binding/airgradient/internal/AirGradientBindingConstants.java                   |    1 
 main/java/org/openhab/binding/airgradient/internal/discovery/AirGradientLocationDiscoveryService.java |    5 +++
 main/java/org/openhab/binding/airgradient/internal/discovery/AirGradientMDNSDiscoveryParticipant.java |    4 ++
 main/java/org/openhab/binding/airgradient/internal/handler/AirGradientAPIHandler.java                 |    2 -
 main/java/org/openhab/binding/airgradient/internal/handler/AirGradientLocationHandler.java            |   53 +++++--------------------------------
 main/java/org/openhab/binding/airgradient/internal/handler/MeasureHelper.java                         |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 main/java/org/openhab/binding/airgradient/internal/model/Measure.java                                 |   22 +++++++++++++++
 main/resources/OH-INF/thing/thing-types.xml                                                           |    1 
 test/java/org/openhab/binding/airgradient/internal/handler/AirGradientLocationHandlerTest.java        |    2 -
 9 files changed, 155 insertions(+), 48 deletions(-)

The local thing:

 java/org/openhab/binding/airgradient/internal/AirGradientBindingConstants.java                   |    1 +
 java/org/openhab/binding/airgradient/internal/AirGradientHandlerFactory.java                     |    9 ++++++++-
 java/org/openhab/binding/airgradient/internal/discovery/AirGradientMDNSDiscoveryParticipant.java |    6 +++---
 resources/OH-INF/thing/thing-types.xml                                                           |   52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 61 insertions(+), 7 deletions(-)

Both PRs are ready, but I dont want to feature creep this review unless you are OK with it.

@lsiepel
Copy link
Contributor

lsiepel commented May 8, 2024

The one with model property and separating out creating of states from the controller (for reuse):

austvik@austvik-desktop:~/src/openhab-addons$` git show 6d32039c9f111aeeec2466bf25ead30d3cb5f357 | diffstat
 main/java/org/openhab/binding/airgradient/internal/AirGradientBindingConstants.java                   |    1 
 main/java/org/openhab/binding/airgradient/internal/discovery/AirGradientLocationDiscoveryService.java |    5 +++
 main/java/org/openhab/binding/airgradient/internal/discovery/AirGradientMDNSDiscoveryParticipant.java |    4 ++
 main/java/org/openhab/binding/airgradient/internal/handler/AirGradientAPIHandler.java                 |    2 -
 main/java/org/openhab/binding/airgradient/internal/handler/AirGradientLocationHandler.java            |   53 +++++--------------------------------
 main/java/org/openhab/binding/airgradient/internal/handler/MeasureHelper.java                         |  113 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 main/java/org/openhab/binding/airgradient/internal/model/Measure.java                                 |   22 +++++++++++++++
 main/resources/OH-INF/thing/thing-types.xml                                                           |    1 
 test/java/org/openhab/binding/airgradient/internal/handler/AirGradientLocationHandlerTest.java        |    2 -
 9 files changed, 155 insertions(+), 48 deletions(-)

The local thing:

 java/org/openhab/binding/airgradient/internal/AirGradientBindingConstants.java                   |    1 +
 java/org/openhab/binding/airgradient/internal/AirGradientHandlerFactory.java                     |    9 ++++++++-
 java/org/openhab/binding/airgradient/internal/discovery/AirGradientMDNSDiscoveryParticipant.java |    6 +++---
 resources/OH-INF/thing/thing-types.xml                                                           |   52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 61 insertions(+), 7 deletions(-)

Both PRs are ready, but I dont want to feature creep this review unless you are OK with it.

It’s ok, just add it to this pr

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@austvik
Copy link
Contributor Author

austvik commented May 8, 2024

Thank you! I'm sorry for adding more. My only defense is that May in Norway is filled with vacation days, and some of them has had bad weather :)

austvik and others added 4 commits May 8, 2024 22:42
…hab/binding/airgradient/internal/discovery/AirGradientLocationDiscoveryService.java

Co-authored-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/discovery/AirGradientLocationDiscoveryService.java

Co-authored-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/discovery/AirGradientMDNSDiscoveryParticipant.java

Co-authored-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
…hab/binding/airgradient/internal/discovery/AirGradientLocationDiscoveryService.java

Co-authored-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@austvik
Copy link
Contributor Author

austvik commented May 8, 2024

Thank you, @jlaur!

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question and some very minor last comments. I think these are the last ones.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
@lsiepel lsiepel merged commit 6efe62f into openhab:main May 9, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.2 milestone May 9, 2024
@lsiepel
Copy link
Contributor

lsiepel commented May 9, 2024

Thanks for you contribution!

Now, you could add your binding's logo to the openHAB website. See https://www.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website

@austvik
Copy link
Contributor Author

austvik commented May 10, 2024

Thank you for great reviews @lsiepel and @jlaur. It was a great learning experience for me, and the end result was much much better than the initial submission!

@austvik
Copy link
Contributor Author

austvik commented May 12, 2024

openhab/openhab-docs#2296 Logo pull request

PRosenb pushed a commit to PRosenb/openhab-addons that referenced this pull request May 22, 2024
* [airgradient] Initial contribution

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
PRosenb pushed a commit to PRosenb/openhab-addons that referenced this pull request May 22, 2024
* [airgradient] Initial contribution

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Jun 15, 2024
* [airgradient] Initial contribution

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Signed-off-by: Paul Smedley <paul@smedley.id.au>
pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
* [airgradient] Initial contribution

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
* [airgradient] Initial contribution

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
* [airgradient] Initial contribution

AirGradient are open source and open hardware air quality sensors that
you can read values from through a cloud API or directly from the device.

Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants