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

[withings] Initial contribution (#4395) #9154

Closed
wants to merge 18 commits into from
Closed

[withings] Initial contribution (#4395) #9154

wants to merge 18 commits into from

Conversation

Novanic
Copy link
Contributor

@Novanic Novanic commented Nov 27, 2020

This pull-request (re-)adds support for Withings devices and data. There was already a binding with OpenHAB 1 which isn't working with OpenHAB 2 anymore. This binding is now a new rewritten binding for OpenHAB 3 which uses newer features like things and discovery.

This first version supports scales and sleep monitors.

See issue #4395 for more information.

Novanic and others added 8 commits October 3, 2020 23:14
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
@Novanic Novanic mentioned this pull request Nov 27, 2020
@Novanic Novanic marked this pull request as draft November 27, 2020 21:57
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
@Novanic Novanic marked this pull request as ready for review November 28, 2020 06:32
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
@chris400
Copy link

(pls don't blame me, I am not a developer)
Might it be possible to fire a trigger, when the person starts to sleep? I did some digging in the Withings API, but did not find it.

@fwolter fwolter added new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2 labels Nov 28, 2020
@Novanic
Copy link
Contributor Author

Novanic commented Nov 29, 2020

(pls don't blame me, I am not a developer)
Might it be possible to fire a trigger, when the person starts to sleep? I did some digging in the Withings API, but did not find it.

Unfortunately not (yet). That would require that the OpenHAB installation is public accessible via the internet. Without that, the Withings API does provide the sleep start information unfortunately only after the sleep session is ended (when the user stands up after sleeping).

Copy link
Member

@fwolter fwolter 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! I reviewed your code and here is my feedback.

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

bundles/org.openhab.binding.withings/NOTICE Outdated Show resolved Hide resolved

### 2. Bridge Configuration

- Create a new Withings API Thing with Paper UI
Copy link
Member

Choose a reason for hiding this comment

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

Can you express this more abstract, as PaperUI will be removed in OH3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, and what is the name of the new configuration UI?

Copy link
Member

Choose a reason for hiding this comment

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

Main UI. But it's better to express it abstractly, that no mention of a specific UI is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I changed that to Main UI. As I know there is currently no other configuration/setup UI and I wouldn't recommend to configure the things with the configuration files, because it would be complicated and the users would have to remove the auth-code manually at the right time, that may cause problems. That problem causes still many support questions at the Innogy binding.

public class WithingsHandlerFactory extends BaseThingHandlerFactory {

private final Logger logger = LoggerFactory.getLogger(WithingsHandlerFactory.class);
private final Map<ThingUID, @Nullable ServiceRegistration<?>> discoveryServiceRegs = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

You could use the ThingHandlerService to make discovery registration code simpler. See the yet unmerged documentation: https://github.com/openhab/openhab-docs/pull/1262/files#diff-c4a4d8725430bc2ea046182bfc73ac51349d989c7f6a8a6fa3001a226a09ad98R932

Copy link
Contributor Author

@Novanic Novanic Dec 5, 2020

Choose a reason for hiding this comment

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

Hm, there is no binding / example which already uses that and the documentation doesn't help me to imagine how can help that to simplify it. :-(

Copy link
Member

Choose a reason for hiding this comment

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

There are several addons using it. E.g. the LCN binding.

bundles/pom.xml Outdated Show resolved Hide resolved
@Novanic Novanic requested a review from a team as a code owner December 4, 2020 20:17
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>
Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com>

### 1. Application Creation

Register a new Withings application here: https://account.withings.com/partner/add_oauth2
Copy link
Contributor

Choose a reason for hiding this comment

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

How can I create an application there? What is the Callback URL? Everyone needs to register openHAB as an application at this site?

@Novanic Novanic marked this pull request as draft December 13, 2020 19:35
@wborn wborn added the bounty Issues with a Bountysource reward and the PRs that solve these label Dec 15, 2020
@wborn wborn linked an issue Dec 15, 2020 that may be closed by this pull request
@Novanic
Copy link
Contributor Author

Novanic commented Dec 20, 2020

I'm sorry but I recall the pull-request for the moment, because...

  • ... there seems to be less interest in that binding
  • ... there is no comment from the author of issue Add Withings Binding #4395 / if it is sufficient to fulfill the requirements
  • ... the setup process is a little bit annoying and can not get improved yet (has to work without a public accessible OpenHAB installation and the authorize link can not get generated dynamically). At least this pain appears only one time, so it could be ok for a first version. Maybe that can get improved in the future within OpenHAB and with OpenHAB-Cloud.
  • ... but the main reason is that I get no sufficient answer from Withings if the approach is ok to let the users registrating there own application (with client id and client secret) at Withings. There are other applications which use the same approach (for example the OpenHAB v1 binding and "Home Assistant") but probably the official way is that 1 application uses 1 client id. That wouldn't work in this case because the secret can not get saved secure in an open-source project and also the quotas wouldn't be sufficient. Therefore I want to get a confirmation from Withings that the approach is ok to don't cause any risk.

@Novanic Novanic closed this Dec 20, 2020
@chris400
Copy link

I am interested in the binding! :)
But I do not have much to provide within its development, so I did not comment on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty Issues with a Bountysource reward and the PRs that solve these new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Withings Binding
6 participants