[Davis] Enable IP communication with davis weather station #5227
Conversation
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.
Thanks for this.
I have a few comments on changes I'd like to have in the design.
First, I feel like the type
setting should be eliminated. The binding can choose whether to use serial or IP based on the whether the hostname
setting is configured.
The configuration settings should be streamlined to have only 1 of each in the user-facing configuration. Eg. instead of having port
and portNumber
, only port
should exist.
I'd also suggest renaming the sleepResponse
setting to "readResponseWaitTime", to better reflect what it does.
Please also ensure you've run all the code through the Eclipse formatter.
Hi, I just implemented your suggestion. I suppress the port number property as it is a fixed value from the manufacturer. I suppressed the 'type' property and rename the other one. One question, what do mean by 'run all the code through the Eclipse formatter' as the option 'formatting' was already activated. Do you mean all other java classes that I didn't modify ? Best regards |
Hi,
I made a commit this morning according to your remarks.
Formatter was already set in eclipse (profile ESH).
Best regards,
Eric
… Le 25 juin 2017 à 04:20, Chris Carman ***@***.***> a écrit :
@9037568 requested changes on this pull request.
Thanks for this.
I have a few comments on changes I'd like to have in the design.
First, I feel like the type setting should be eliminated. The binding can choose whether to use serial or IP based on the whether the hostname setting is configured.
The configuration settings should be streamlined to have only 1 of each in the user-facing configuration. Eg. instead of having port and portNumber, only port should exist.
I'd also suggest renaming the sleepResponse setting to "readResponseWaitTime", to better reflect what it does.
Please also ensure you've run all the code through the Eclipse formatter.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#5227 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ASoRJW3GyhTSpnZtFoCTRX_pByYjnLN2ks5sHcPagaJpZM4OEOOl>.
|
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.
Thanks for the updates. Just a few more issues that need fixing.
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.davis/src/main/java/org/openhab/binding/davis/internal/DavisBinding.java
Outdated
Show resolved
Hide resolved
I just made a commit but it will be the last, I switch to Jeedom solution last month, it is by far more reactive and mature. The fact of wiping off Eclipse editor is an error for me (I'm a Mac user). Bye |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/davis-weather-via-ip/43429/1 |
Thanks, @enricobaros ! |
Hi,
I just made a push of my work. This bundle is working fine for about 2 weeks in my openhab2 installation.
Goal of this enhancement is to communicate by IP (new) or SERIAL (existing) with Davis weather station. The protocol remains the same.
Best regards