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

[flume] Initial contribution #17152

Merged
merged 10 commits into from
Sep 8, 2024
Merged

[flume] Initial contribution #17152

merged 10 commits into from
Sep 8, 2024

Conversation

jsjames
Copy link
Contributor

@jsjames jsjames commented Jul 25, 2024

This is a new binding to read water usage when using a flume water meter device (https://flumewater.com/). It utilizes the Flume cloud API (https://flumetech.readme.io/reference/introduction).

A built version of the binding is at this location: https://drive.google.com/file/d/13lnVOnUnVKCdudOXF7crdGdTNEL_ACW6/view?usp=drive_link

@jsjames jsjames added the new binding If someone has started to work on a binding. For a new binding PR. label Jul 25, 2024
@jsjames jsjames requested a review from a team as a code owner July 25, 2024 19:43
@jsjames jsjames force-pushed the flume branch 2 times, most recently from ee05c4d to 2ed6c2c Compare July 25, 2024 20:51
@lsiepel
Copy link
Contributor

lsiepel commented Aug 1, 2024

Thank you for this contribution. As there are many PR’s to review, it would nice if you can do some self review upfront. A checklist is here https://github.com/openhab/openhab-addons/wiki/Review-Checklist

I noticed that the channel id’s and maybe more do not match the naming convention. For reference https://www.openhab.org/docs/developer/guidelines.html#naming-convention

Please ping me when you are ready.

@jsjames jsjames force-pushed the flume branch 2 times, most recently from b27fda9 to c6cfa23 Compare August 1, 2024 15:07
@jsjames
Copy link
Contributor Author

jsjames commented Aug 1, 2024

Thank you for this contribution. As there are many PR’s to review, it would nice if you can do some self review upfront. A checklist is here https://github.com/openhab/openhab-addons/wiki/Review-Checklist

I noticed that the channel id’s and maybe more do not match the naming convention. For reference https://www.openhab.org/docs/developer/guidelines.html#naming-convention

Please ping me when you are ready.

@lsiepel - I addressed the naming convention in this commit and I did go back through the checklist/code guidelines and "believe" everything is in a good state.

@jlaur jlaur added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Aug 1, 2024
@digitaldan
Copy link
Contributor

Thanks for the binding @jsjames , my flume2 comes today so very excited to hook it up to openHAB and tie it in with my sprinkler monitoring (Hydrawsie binding) . I'll let you know how it goes !

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 again for this contribution. Just finish this partial review, looked at 25/32 files. Looks pretty good, some comments and questions for now. Hope to finish this review later this week.

@jsjames
Copy link
Contributor Author

jsjames commented Aug 19, 2024

Thanks for the binding @jsjames , my flume2 comes today so very excited to hook it up to openHAB and tie it in with my sprinkler monitoring (Hydrawsie binding) . I'll let you know how it goes !

@digitaldan, it will be good to know how you end up linking them. I am using the rachio binding which notified when a zone is turned on/off. I persist both that and the cumulative water usage into influx. Then, I use a "very complex" flux query to retrieve the data. Its somewhat error prone and influxdb is deprecating the flux language - so i'm planning to use a different method. Hope the binding will work well for you.

@jsjames jsjames force-pushed the flume branch 2 times, most recently from 39712d7 to e364faa Compare August 20, 2024 04:20
@lsiepel lsiepel changed the title [flume] Flume water meter binding submission [flume] Initial contribution Aug 20, 2024
Signed-off-by: Jeff James <[email protected]>
Signed-off-by: Jeff James <[email protected]>
Signed-off-by: Jeff James <[email protected]>
fixed issue with multiple notifications

Signed-off-by: Jeff James <[email protected]>
Signed-off-by: Jeff James <[email protected]>
Streamlined some iterations to use stream

Signed-off-by: Jeff James <[email protected]>
Signed-off-by: Jeff James <[email protected]>
addressed issues in design review

Signed-off-by: Jeff James <[email protected]>
@jsjames
Copy link
Contributor Author

jsjames commented Sep 8, 2024

Last part of the review. Nice to see all features like i18n are in place. Thanks again. Left some questions and comments. Ping me when you are ready.

@lsiepel - I marked the items resolved, but have to still push the changes into the repository. Was going to let it run tonight to confirm everything works and will push up the changes tomorrow. Anyway, don't merge changes quite yet. thanks.

Yes I know still have to verify the changes.

Sounds good, i just saw you marked as approved. Should be good to go now. I streamlined/refactored some of the API calls, so there are quite a few changes to that file - but it is better structured.

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.

Very minor comment and the i18n plugin has to be re-run. Otherwise LGTM.

@jsjames jsjames added the rebuild Triggers Jenkins PR build label Sep 8, 2024
@lsiepel lsiepel removed the rebuild Triggers Jenkins PR build label Sep 8, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Sep 8, 2024

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

@lsiepel lsiepel merged commit 8f62374 into openhab:main Sep 8, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Sep 8, 2024
@jsjames jsjames deleted the flume branch September 8, 2024 20:48
@jsjames
Copy link
Contributor Author

jsjames commented Sep 8, 2024

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

@lsiepel - Thanks for all you do to make us all better coders!!

digitaldan pushed a commit to digitaldan/openhab-addons that referenced this pull request Sep 24, 2024
* Initial submission

Signed-off-by: Jeff James <[email protected]>
pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
* Initial submission

Signed-off-by: Jeff James <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
* Initial submission

Signed-off-by: Jeff James <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
* Initial submission

Signed-off-by: Jeff James <[email protected]>
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.

4 participants