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

[iotawatt] Initial contribution #16491

Merged
merged 40 commits into from
May 28, 2024
Merged

Conversation

PRosenb
Copy link
Contributor

@PRosenb PRosenb commented Mar 6, 2024

This PR adds a new binding for IoTaWatt, an open source power measuring device.

Limitations of this version:

  • No authentication support

Short discussion about the topic: Forum thread IoTaWatt

Any kinds of feedback is welcome, thank you.

Snapshot Download

@PRosenb PRosenb changed the title New binding io ta watt [iotawatt] New binding for IoTaWatt Mar 6, 2024
@PRosenb PRosenb changed the title [iotawatt] New binding for IoTaWatt [iotawatt][WIP] Initial contribution Mar 6, 2024
@jlaur jlaur added the new binding If someone has started to work on a binding. For a new binding PR. label Mar 6, 2024
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/iotawatt/153849/3

@PRosenb PRosenb force-pushed the NewBindingIoTaWatt branch 2 times, most recently from 4bc5416 to 17eb8d7 Compare March 24, 2024 12:02
@PRosenb PRosenb marked this pull request as ready for review March 26, 2024 11:00
@PRosenb PRosenb requested a review from a team as a code owner March 26, 2024 11:00
@PRosenb PRosenb changed the title [iotawatt][WIP] Initial contribution [iotawatt] Initial contribution May 1, 2024
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.

Thansk for this contribution. In general it seems already solid. Besides the minor comments, i guess there are two areas that i have questions about:

  1. exception handling / thing status details, see comments.
  2. thing-types. I see several power/energy rel;ated channels that have diffent quantitytypes declared compare to others bindings that have the same channels. Please double check.

@PRosenb PRosenb force-pushed the NewBindingIoTaWatt branch 3 times, most recently from a03441a to b7a6564 Compare May 7, 2024 12:44
@PRosenb
Copy link
Contributor Author

PRosenb commented May 19, 2024

Many thanks @lsiepel for reviewing my PR by the way.

2. thing-types. I see several power/energy rel;ated channels that have diffent quantitytypes declared compare to others bindings that have the same channels. Please double check.

When I implemented the types, I was also not sure which ones to use. I had a look at the Shelly binding because I use it and it has quite a lot of types. Which one did you compare it to? What do you feel should it be changed to?
I hope we can get them right.

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.

Final comments added. According to the docs, the power related channels types are good. Only the units should be replace by a placeholder.

Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
@PRosenb
Copy link
Contributor Author

PRosenb commented May 22, 2024

@lsiepel I have issues with %unit% and mentioned it on your comment, but unfortunately is it hidden now.
Do you have a clue why it's not working, at least on my set up?

@lsiepel
Copy link
Contributor

lsiepel commented May 24, 2024

@lsiepel I have issues with %unit% and mentioned it on your comment, but unfortunately is it hidden now. Do you have a clue why it's not working, at least on my set up?

Hmm maybe try %% as unit. i have to search, but percentages have a different handling since a while. You can also check some otherbinding setting a percentage value.

Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>
@PRosenb
Copy link
Contributor Author

PRosenb commented May 25, 2024

Thanks for your hints @lsiepel and @jlaur. It was not an issue of the channel-type definition but only of the item definition in the readme where the dimension was missing. Now it works like a charm.

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 your contribution. LGTM.
Let me know once you think this binding is tested enough to merge.

@PRosenb
Copy link
Contributor Author

PRosenb commented May 28, 2024

Thanks for your contribution. LGTM. Let me know once you think this binding is tested enough to merge.

Many thanks for your review and good inputs @lsiepel. I tested it on a test setup but haven't yet installed an IoTaWatt on my switchboard because I need an electrician for that. I will have it installed in the coming weeks, set it up to monitor the whole house and integrate it into my rules. Should I find bugs at that stage I'll submit bug fixes.

@lsiepel lsiepel merged commit a4ad7b2 into openhab:main May 28, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.2 milestone May 28, 2024
@lsiepel
Copy link
Contributor

lsiepel commented May 28, 2024

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

Please also see the updated documentation here - SVG is now preferred:
https://next.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website-and-the-ui

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.

None yet

5 participants