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

[smgw] Initial contribution #16017

Merged
merged 5 commits into from
Dec 17, 2023
Merged

[smgw] Initial contribution #16017

merged 5 commits into from
Dec 17, 2023

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Dec 7, 2023

This integrates smart meter gateways from PPC which are in widespread-use for supplier provided smart meters.

Topic: https://community.openhab.org/t/integration-smart-meter-geteway-smgw-in-openhab/123772
Marketplace: https://community.openhab.org/t/smgw-binding-4-0-0-5-0-0/151845

@J-N-K J-N-K added new binding If someone has started to work on a binding. For a new binding PR. work in progress A PR that is not yet ready to be merged labels Dec 7, 2023
@J-N-K J-N-K requested a review from a team as a code owner December 7, 2023 18:04
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K removed the work in progress A PR that is not yet ready to be merged label Dec 14, 2023
@lolodomo
Copy link
Contributor

Only 522 lines !
I am tempted to try an express review...

@J-N-K
Copy link
Member Author

J-N-K commented Dec 16, 2023

It's quite simple, but hard to do in a rule because you need to get cookies and headers right. Otherwise it's just some website parsing.

Signed-off-by: Jan N. Klug <github@klug.nrw>
Copy link
Member Author

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

All done.

Signed-off-by: Jan N. Klug <github@klug.nrw>
@lolodomo
Copy link
Contributor

lolodomo commented Dec 17, 2023

@J-N-K : you forgot the comment about the NOTICE file. You can take example from other bindings using jsoup. I also just added a last minor comment about binding naming in the documentation.

For example:
https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.linky/NOTICE#L15

Signed-off-by: Jan N. Klug <github@klug.nrw>
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

Thank your this new binding contribution.

@lolodomo lolodomo merged commit 3e0deab into openhab:main Dec 17, 2023
3 checks passed
@lolodomo lolodomo added this to the 4.1 milestone Dec 17, 2023
@J-N-K J-N-K deleted the smgw branch December 17, 2023 09:17
@stefan-hoehn
Copy link
Contributor

@J-N-K out of curiosity: Doesn't a bundle require a specific image in openhab-docs/images/addons or did I overlook the PR for that?

@J-N-K
Copy link
Member Author

J-N-K commented Dec 17, 2023

@stefan-hoehn What is the policy regarding the license of the pictures?

PPC provides the logo for download, but unfortunately there is no hint to the license. https://www.ppc-ag.de/de/unternehmen/downloadcenter/

@stefan-hoehn
Copy link
Contributor

This is actually a pretty good question which I never thought of.

We have not written anything about that here https://www.openhab.org/docs/developer/addons/ either.

I would relate it as "fair use" but I am sure that this is legally a wrong perception. "most likely" all the companies we write a binding for will not mind but again that is not a statement that sticks in the long run.

The only way to be sure is to ask the company for their allowance, I guess.

@kaikreuzer @Confectrician
Any thoughts or experience you can share on this?

@jlaur
Copy link
Contributor

jlaur commented Dec 17, 2023

The only way to be sure is to ask the company for their allowance, I guess.

I'm also interested in this. For Energi Data Service and Danfoss Air Unit I have written permissions from the companies to be on the safe side, but it was hard figuring out who to ask and getting through to someone who could actually give that permission. I don't even know how legally valid those e-mailed permissions are, but at least they are proof of "good faith". The other thing is that I personally have those e-mails, but they are not (yet) shared anywhere.

I'm under the impression that many logo contributions were without any kind of explicit permissions, so I guess it would be good to have guidelines for what is expected (or what is not).

@kaikreuzer
Copy link
Member

kaikreuzer commented Dec 18, 2023

We have thought about this already. Initially, we avoided using such logos for exactly these reasons. That's why openHAB 1 and 2 looked very dull in comparison to today's UI. 😄

We then had a look over the fence to Home Assistant and saw that they had gathered a huge pile of logos in their repo and didn't seem to care much about explicit permission, so we decided to do the same.

To make sure that we do not appear to publish these logos as if we would own them, we explicitly excluded them from our license and made it very clear that they are property of their respective owners.

It is naturally the right of every company to ask us not to use their logo - we will remove it from the repo immediately in such a case. So far, this has never happened though, so we believe that everybody silently agrees that this is fair use and that it actually benefits all sides.

@stefan-hoehn
Copy link
Contributor

@J-N-K Would you like to add the logo then?

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
Signed-off-by: Jan N. Klug <github@klug.nrw>
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