[bticinosmarther] Initial contribution #7533

MrRonfo · 2020-05-02T19:18:54Z

[bticinosmarther] Initial contribution

The BTicino Smarther binding implements a bridge to the C2C Legrand/Bticino Smarther v2.0 API and allows users to control their BTicino Smarther chronothermostat units (all models) with openHAB.

BTicino Smarther units have their own API set and doesn't conform to the OpenWebNet protocol.

Main thread in openHAB Community on this binding:

https://community.openhab.org/t/bticino-smarther-thermostat-binding/73329

Companion HABPanel widget for this binding:

https://community.openhab.org/t/custom-widget-bticino-smarther-widget/97522

Other threads in openHAB Community where the idea to build this binding came from:

Signed-off-by: Fabio Possieri [email protected]

TravisBuddy · 2020-05-02T19:39:01Z

Travis tests were successful

Hey @MrRonfo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

TravisBuddy · 2020-05-03T12:13:26Z

Travis tests were successful

Hey @MrRonfo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

TravisBuddy · 2020-05-06T18:59:10Z

Travis tests were successful

Hey @MrRonfo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

fwolter

You put a lot of effort into this binding. There are only minor improvements, I found.

I'm not that familiar with servlets. Another reviewer should take a closer look into this part.

bundles/org.openhab.binding.smarther/README.md

bundles/org.openhab.binding.smarther/src/main/resources/ESH-INF/binding/binding.xml

bundles/pom.xml

...java/org/openhab/binding/smarther/internal/api/exception/SmartherAuthorizationException.java

.../main/java/org/openhab/binding/smarther/internal/api/exception/SmartherGatewayException.java

.../java/org/openhab/binding/smarther/internal/api/exception/SmartherNotificationException.java

MrRonfo

Thanks @fwolter for your kind suggestions, I'll try to review them by today.

MrRonfo · 2020-05-25T19:03:24Z

Hi @fwolter, after having accepted few of your PR reviews here via github I found more practical to apply all of them locally, review and just commit once.
However, I noted that I cannot git push from local to my remote branch (as non-fast-forward), since the few commits above have been automatically reported by github to my remote branch too.
Any suggestions ?
Would a git push --force work in this case or is it better to act differently ?
Thanks!

fwolter · 2020-05-25T20:05:43Z

Before you can push your changes, you have to rebase your local branch to incorporate your changes you done via the GitHub webinterface.

MrRonfo · 2020-05-25T21:49:10Z

@fwolter thanks, done but had signoff issue, so DCO suggested to resolve with:

git rebase HEAD~11 --signoff
git push --force-with-lease origin smarther

Applied on my local copy but I think it messed up something, as I can see commits added for other bindings which actually weren't on my branch :|
How should I better proceed with that? Any way I can recover or what else?

Sorry to the other guys over there I accidentally requested a review to...

TravisBuddy · 2020-05-25T21:59:10Z

Travis tests have failed

Hey @MrRonfo,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

fwolter · 2020-05-25T22:44:08Z

Seems like you did a rebase on an old 2.5.x branch. First, make a backup of your local repo. Then, you can delete the five foreign commits by interactive rebase: git rebase -i e5e1561~1 (this is one commit before the bluetooth commit). Delete one at a time, to prevent conflicts and re-run the interactive rebase. If all looks fine again, you can do a force-push. No guarantees :) Hope that helps.

MrRonfo · 2020-05-26T18:44:28Z

@fwolter thank you so much for your kind support, it seems to me the binding codebase is now ok.
All the reviews have been incorporated in the last commit, apart from the one pending on RuntimeException.
Cheers,

TravisBuddy · 2020-05-26T19:14:28Z

Travis tests have failed

Hey @MrRonfo,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

fwolter

If you rename the package org.openhab.binding.smarther.internal.api.model to org.openhab.binding.smarther.internal.api.dto you'd get rid of several NonNullByDefault warnings.

You repalced the Joda calls with the old Date API. The vast majority of Joda has been merged into Java. You don't have to re-implement e.g. plusDays. Take a look at ZonedDateTime or Instant if UTC is sufficent.

...g.smarther/src/main/java/org/openhab/binding/smarther/internal/api/SmartherApiConnector.java

...ab.binding.smarther/src/main/java/org/openhab/binding/smarther/internal/util/StringUtil.java

MrRonfo · 2020-05-28T18:46:00Z

Thanks again @fwolter for your code review. All suggestions merged and pushed here.

I've noted that Travis CI is failing, it seems, due to my codebase being on OH 2.5.5 while it's now expecting to build agains OH 2.5.6.
Would it be a good idea to rebase my smarther branch to origin/2.5.x (and, in case, what is the standard approach to do that for OH) ?

fwolter · 2020-05-28T20:31:23Z

Correct, rebase will do the job.

MrRonfo · 2020-05-29T15:00:24Z

Correct, rebase will do the job.

@fwolter noted, thanks. So no risk the whole locally rebased code of origin/2.5.x to be also added to this PR, when I'll push new code fixes to my remote Smarther branch ?
Sorry for the trivial question, but I don't want to mess anything like I did in past commits :)

fwolter · 2020-05-29T15:47:24Z

Yes. If you're unsure, make a backup of your local repo. If you messed up something you can do a force-push from your backup, although it's not elegant.

TravisBuddy · 2020-05-30T08:57:43Z

Travis tests were successful

Hey @MrRonfo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Fixed the Bridge thing transition to Unknown status in initialize method, moving it after variables initialization so that handleCommand() is eventually called only after. Signed-off-by: Fabio Possieri <[email protected]>

Changed wrong use of NonNullByDefault with Nullable wherever found, removed OSGI component declaration in ThingHandlerService, fixed unit attribute in config.xml Signed-off-by: Fabio Possieri <[email protected]>

Signed-off-by: Fabio Possieri <[email protected]>

Added syntax exception handling to Json parsing calls, optimized reading new notification from servlet request, fixed module discovery service. Signed-off-by: Fabio Possieri <[email protected]>

Just refactored the binding naming to bticinosmarther (smarther alone was too generic), as BTicino also provides other protocols that can be integrated over time in OH by other developers Signed-off-by: Fabio Possieri <[email protected]>

TravisBuddy · 2020-06-23T20:54:40Z

Travis tests have failed

Hey @MrRonfo,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

Signed-off-by: Fabio Possieri <[email protected]>

TravisBuddy · 2020-06-23T21:32:50Z

Travis tests were successful

Hey @MrRonfo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

MrRonfo · 2020-06-24T06:46:30Z

@cpmeister @fwolter
Hi guys, I've updated the PR files to OH 2.5.7 and just taken the opportunity to rename the binding from "smarther" to "bticinosmarther" for two reasons:

I found "smarther" to be a too generic naming (other devices in the market have similar names, this way is more easy for the end user to identify the right one);
BTicino also provides other protocols to manage different systems (so, in the hope to keep some namespace consistency in case someone else will integrate one of these with OH in the future).

I see "changes approved": does that mean that the binding can be merged or is there anything else to be done here?
Thanks!

kaikreuzer · 2020-06-24T09:04:58Z

@MrRonfo The renaming makes sense to me, thanks.
I have a few final comments, which I will post tonight (don't have time earlier than that), but then we should be good to merge!

kaikreuzer

Thanks for your patience, @MrRonfo, here are my promised comments :-)

bundles/org.openhab.binding.bticinosmarther/README.md

MrRonfo · 2020-06-26T14:50:45Z

@kaikreuzer thanks for your suggestions, really appreciated. I've added few questions. I'll commit all the changes as soon as I get your feedback.
Cheers,

kaikreuzer · 2020-06-27T09:38:36Z

Done - I hope I answered all questions. If I have missed any, please let me know :-)

Aligned README to OH template, fixed config locations fetch channel for Bridge thing and config programs fetch channel for Module thing, moved accessToken info from channel to log at trace level. Signed-off-by: Fabio Possieri <[email protected]>

TravisBuddy · 2020-06-27T16:25:41Z

Travis tests were successful

Hey @MrRonfo,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

MrRonfo · 2020-06-27T19:27:48Z

Done - I hope I answered all questions. If I have missed any, please let me know :-)

Thanks a lot @kaikreuzer.
I've integrated all of your suggestions to the last commit. Also changed from the generic (and a bit misleading) fetchConfig in both SmartherBridgeHandler and SmartherModuleHandler, to specific fetchLocations and fetchPrograms respectively.
Cheers,

kaikreuzer

Thanks for your very quick updates, @MrRonfo - lgtm now, so let's merge! 👍

Signed-off-by: Fabio Possieri <[email protected]>

Signed-off-by: Fabio Possieri <[email protected]> Signed-off-by: CSchlipp <[email protected]>

Signed-off-by: Fabio Possieri <[email protected]> Signed-off-by: MPH80 <[email protected]>

Signed-off-by: Fabio Possieri <[email protected]>

Signed-off-by: Fabio Possieri <[email protected]> Signed-off-by: Daan Meijer <[email protected]>

Signed-off-by: Fabio Possieri <[email protected]>

MrRonfo requested a review from a team as a code owner May 2, 2020 19:18

cpmeister added the new binding If someone has started to work on a binding. For a new binding PR. label May 3, 2020

fwolter requested changes May 23, 2020

View reviewed changes

MrRonfo commented May 24, 2020

View reviewed changes

MrRonfo force-pushed the smarther branch from 2b5d651 to ae26de1 Compare May 25, 2020 21:42

MrRonfo requested review from cdjackson, cpmeister, cweitkamp, kaikreuzer, paulianttila and ssalonen as code owners May 25, 2020 21:42

MrRonfo force-pushed the smarther branch from ae26de1 to 691a6fb Compare May 26, 2020 18:38

MrRonfo removed the request for review from a team May 26, 2020 18:50

fwolter requested changes May 27, 2020

View reviewed changes

MrRonfo force-pushed the smarther branch from e510d2e to 5c671cc Compare May 30, 2020 08:54

MrRonfo added 5 commits June 23, 2020 20:40

Fixed transition to Unknown status on Bridge thing

67300fb

Fixed the Bridge thing transition to Unknown status in initialize method, moving it after variables initialization so that handleCommand() is eventually called only after. Signed-off-by: Fabio Possieri <[email protected]>

Some fixes applied following PR Review from cpmeister.

d8631ae

Changed wrong use of NonNullByDefault with Nullable wherever found, removed OSGI component declaration in ThingHandlerService, fixed unit attribute in config.xml Signed-off-by: Fabio Possieri <[email protected]>

Some fixes applied following PR Review from cpmeister.

2955530

Signed-off-by: Fabio Possieri <[email protected]>

Some fixes applied following PR Review from cpmeister.

0d8ef84

Added syntax exception handling to Json parsing calls, optimized reading new notification from servlet request, fixed module discovery service. Signed-off-by: Fabio Possieri <[email protected]>

MrRonfo force-pushed the smarther branch from 38395cb to 1813b1e Compare June 23, 2020 20:52

Aligned pom.xml to OH 2.5.7 snapshot

8bf252f

Signed-off-by: Fabio Possieri <[email protected]>

MrRonfo changed the title ~~[smarther] Initial contribution~~ [bticinosmarther] Initial contribution Jun 24, 2020

kaikreuzer requested changes Jun 24, 2020

View reviewed changes

kaikreuzer approved these changes Jun 28, 2020

View reviewed changes

kaikreuzer merged commit 8a5c07f into openhab:2.5.x Jun 28, 2020

cpmeister added this to the 2.5.7 milestone Jun 29, 2020

knikhilwiz pushed a commit to knikhilwiz/openhab2-addons that referenced this pull request Jul 12, 2020

[bticinosmarther] Initial contribution (openhab#7533)

4ba2aab

Signed-off-by: Fabio Possieri <[email protected]>

CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020

[bticinosmarther] Initial contribution (openhab#7533)

0e567c3

Signed-off-by: Fabio Possieri <[email protected]> Signed-off-by: CSchlipp <[email protected]>

MPH80 pushed a commit to MPH80/openhab-addons that referenced this pull request Aug 3, 2020

[bticinosmarther] Initial contribution (openhab#7533)

8210ac9

Signed-off-by: Fabio Possieri <[email protected]> Signed-off-by: MPH80 <[email protected]>

andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020

[bticinosmarther] Initial contribution (openhab#7533)

d3ea3e7

Signed-off-by: Fabio Possieri <[email protected]>

andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020

[bticinosmarther] Initial contribution (openhab#7533)

f4a6a17

Signed-off-by: Fabio Possieri <[email protected]>

andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020

[bticinosmarther] Initial contribution (openhab#7533)

3f71281

Signed-off-by: Fabio Possieri <[email protected]>

andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020

[bticinosmarther] Initial contribution (openhab#7533)

d4eb831

Signed-off-by: Fabio Possieri <[email protected]>

DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020

[bticinosmarther] Initial contribution (openhab#7533)

2a8b0d8

Signed-off-by: Fabio Possieri <[email protected]> Signed-off-by: Daan Meijer <[email protected]>

markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020

[bticinosmarther] Initial contribution (openhab#7533)

55adbb0

Signed-off-by: Fabio Possieri <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bticinosmarther] Initial contribution #7533

[bticinosmarther] Initial contribution #7533

MrRonfo commented May 2, 2020 •

edited

Loading

TravisBuddy commented May 2, 2020

TravisBuddy commented May 3, 2020

TravisBuddy commented May 6, 2020

fwolter left a comment

MrRonfo left a comment •

edited

Loading

MrRonfo commented May 25, 2020 •

edited

Loading

fwolter commented May 25, 2020

MrRonfo commented May 25, 2020 •

edited

Loading

TravisBuddy commented May 25, 2020

fwolter commented May 25, 2020

MrRonfo commented May 26, 2020

TravisBuddy commented May 26, 2020

fwolter left a comment

MrRonfo commented May 28, 2020

fwolter commented May 28, 2020

MrRonfo commented May 29, 2020

fwolter commented May 29, 2020

TravisBuddy commented May 30, 2020

TravisBuddy commented Jun 23, 2020

TravisBuddy commented Jun 23, 2020

MrRonfo commented Jun 24, 2020 •

edited

Loading

kaikreuzer commented Jun 24, 2020

kaikreuzer left a comment

MrRonfo commented Jun 26, 2020

kaikreuzer commented Jun 27, 2020

TravisBuddy commented Jun 27, 2020

MrRonfo commented Jun 27, 2020

kaikreuzer left a comment

[bticinosmarther] Initial contribution #7533

[bticinosmarther] Initial contribution #7533

Conversation

MrRonfo commented May 2, 2020 • edited Loading

TravisBuddy commented May 2, 2020

Travis tests were successful

TravisBuddy commented May 3, 2020

Travis tests were successful

TravisBuddy commented May 6, 2020

Travis tests were successful

fwolter left a comment

Choose a reason for hiding this comment

MrRonfo left a comment • edited Loading

Choose a reason for hiding this comment

MrRonfo commented May 25, 2020 • edited Loading

fwolter commented May 25, 2020

MrRonfo commented May 25, 2020 • edited Loading

TravisBuddy commented May 25, 2020

Travis tests have failed

fwolter commented May 25, 2020

MrRonfo commented May 26, 2020

TravisBuddy commented May 26, 2020

Travis tests have failed

fwolter left a comment

Choose a reason for hiding this comment

MrRonfo commented May 28, 2020

fwolter commented May 28, 2020

MrRonfo commented May 29, 2020

fwolter commented May 29, 2020

TravisBuddy commented May 30, 2020

Travis tests were successful

TravisBuddy commented Jun 23, 2020

Travis tests have failed

TravisBuddy commented Jun 23, 2020

Travis tests were successful

MrRonfo commented Jun 24, 2020 • edited Loading

kaikreuzer commented Jun 24, 2020

kaikreuzer left a comment

Choose a reason for hiding this comment

MrRonfo commented Jun 26, 2020

kaikreuzer commented Jun 27, 2020

TravisBuddy commented Jun 27, 2020

Travis tests were successful

MrRonfo commented Jun 27, 2020

kaikreuzer left a comment

Choose a reason for hiding this comment

MrRonfo commented May 2, 2020 •

edited

Loading

MrRonfo left a comment •

edited

Loading

MrRonfo commented May 25, 2020 •

edited

Loading

MrRonfo commented May 25, 2020 •

edited

Loading

MrRonfo commented Jun 24, 2020 •

edited

Loading