-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[radiothermostat] RadioThermostat Binding - initial contribution #7266
Conversation
Signed-off-by: Michael Lobstein <[email protected]>
f656bfe
to
a143408
Compare
Travis tests have failedHey @mlobstein, |
Signed-off-by: Michael Lobstein <[email protected]>
Travis tests were successfulHey @mlobstein, |
…lues to bounce Signed-off-by: Michael Lobstein <[email protected]>
Travis tests were successfulHey @mlobstein, |
The RadioThermostat supports a type of SSDP discovery so you don't need to add each one manually. While stuck at home I wrote a binding for it, and if you want you can take the discovery portion of that or I can try to merge it in with your thing. |
Signed-off-by: Michael Lobstein <[email protected]>
Thanks! I added it to the PR! |
Travis tests were successfulHey @mlobstein, |
Signed-off-by: Michael Lobstein <[email protected]>
update fork
update fork
Signed-off-by: Michael Lobstein <[email protected]>
Travis tests were successfulHey @mlobstein, |
Would it make any sense to augment this:
With something like this: As often as I've been using the RadioThermostat, all I've wanted to do really with these values is store away the runtimes and look at them on a group graph. Having a channel that provides a single number would avoid doing transformations to persist the single value needed. |
Yeah that is a good idea. I was originally thinking in terms of just exposing the data from the json api directly to provide compatibility with rule based scraping of the json data that many have done with this thermostat over the years. That also why I put in the channels to provide the thermostat time from the json data even though it is mostly redundant. What unit of time should this single number be? Minutes perhaps? For heat today, I would do the following (today_heat_hour * 60) + today_heat_minute = RuntimeTodayHeat (minutes). Does that sound good? |
Yeah just
and to store it, you could use
That way, expressed as Number:Time, the user can change the representation as they need. There is one other thing that might be important due to the way these thermostats work. When these guys are in HOLD at a setpoint, you can't transition to a temporary value without unholding it. Other times you may want to set a value and HOLD it. So the way you have things now I think that would be two commands. Since the thermostat can take the json string, I have a simple channel called "command" that just checks and passes that to the thermostat, eg: In this way you avoid the double command, and issues with the radiothermostat timing out when it's doing too much. Also, since people would migrate from the http binding, they are already sending json strings. And this also leaves open the possibility that somebody may want to do something with an undocumented api that we weren't aware of. |
Signed-off-by: Michael Lobstein <[email protected]>
@billfor I made your suggested changes. Try it out and let me know what you think. |
Travis tests were successfulHey @mlobstein, |
Looks good! Thank you.
|
Signed-off-by: Michael Lobstein <[email protected]>
Travis tests were successfulHey @mlobstein, |
While waiting for the next reviewer. Please try to resolve all of these build warnings. Most of them can be resolved by caching fields in local variables and performing your logic on those local variables instead. If there are any you have trouble with let me know and I'll tell you how to solve it.
|
I am not sure about those that all appear because of the NotNullByDefault annotation. In the DTO for example, the only way I see to fix it is to assign default values for everything. |
We have null checker exclusions in place for DTO classes, if you want to have your DTO class excluded from null checks you must fulfill one of the two following criteria:
|
Signed-off-by: Michael Lobstein <[email protected]>
Travis tests were successfulHey @mlobstein, |
update fork
Signed-off-by: Michael Lobstein <[email protected]>
Done! |
|
||
This binding connects RadioThermostat/3M Filtrete models CT30, CT50/3M50, CT80, etc. with built-in Wi-Fi module to openHAB. | ||
|
||
The binding retrieves and periodically updates all basic system information from the thermostat. The main thermostat functions such |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a line break after every sentence throughout the README, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
<parent> | ||
<groupId>org.openhab.addons.bundles</groupId> | ||
<artifactId>org.openhab.addons.reactor.bundles</artifactId> | ||
<version>2.5.6-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As 2.5.6 has just been release, we have to ask you to change this version for a very last time, promised ;-)
<version>2.5.6-SNAPSHOT</version> | |
<version>2.5.7-SNAPSHOT</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Thanks @kaikreuzer
update fork
Signed-off-by: Michael Lobstein <[email protected]>
Travis tests were successfulHey @mlobstein, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, thank you!
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]> Signed-off-by: CSchlipp <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]> Signed-off-by: MPH80 <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]> Signed-off-by: Daan Meijer <[email protected]>
…nhab#7266) Signed-off-by: Michael Lobstein <[email protected]>
This binding connects RadioThermostat/3M Filtrete models CT30, CT50/3M50, CT80, etc. with built-in Wi-Fi module using its HTTP/JSON interface.