Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

Binding for relay boards available from http://www.ucprojects.eu #3082

Merged
merged 1 commit into from Dec 2, 2015

Conversation

rmichalak
Copy link
Contributor

No description provided.

@hakan42
Copy link
Contributor

hakan42 commented Nov 29, 2015

Honestly, the name "relayboard" is too generic for this binding. There are many more relayboards besides the one created by ucprojects (a polish company if I believe google translator)

Could you maybe prefix the name with some company name? I could not find any such information on the web page of ucprojects, though.

@rmichalak
Copy link
Contributor Author

So maybe I can rename it to ucproject-relayboard or ucproject-rb?
Which one you prefer? I will change the name and resolve conflicts once we agree on a name.

Oh, and thanks for digging this out - I thought that this is a long forgotten branch ;)

@hakan42
Copy link
Contributor

hakan42 commented Nov 30, 2015

Please call it ucproject-relayboard and rebase the branch against the current master.

After that, give me a short notice here and I will be happy to continue the review.

@hakan42 hakan42 self-assigned this Nov 30, 2015
@hakan42 hakan42 added this to the 1.8.0 milestone Nov 30, 2015
@rmichalak
Copy link
Contributor Author

Do you want me to rename java package to ucproject-relayboard as well? I would prefer to rename it to ucprelayboard - to not introduce very long package name with '-' in the middle. Same for a class names - I would add UCP prefix to them. What do you think?

@teichsta
Copy link
Member

teichsta commented Dec 1, 2015

I would prefer to rename it to ucprelayboard

+1

@hakan42
Copy link
Contributor

hakan42 commented Dec 1, 2015

Maybe call the package ucprelayboard and the binding ucp-relayboard?

Or, if you like it more, name both the packet and the binding identically ucprelayboard?

@hakan42
Copy link
Contributor

hakan42 commented Dec 1, 2015

And prefixing the classes with UCP is excellent assume well 😄

@teichsta
Copy link
Member

teichsta commented Dec 1, 2015

Maybe call the package ucprelayboard and the binding ucp-relayboard?

if i could choose i would always prefer names without dashes "-"

@hakan42
Copy link
Contributor

hakan42 commented Dec 1, 2015

Ok, without dashes is the common preference then 😄

@rmichalak
Copy link
Contributor Author

Done. Changed binding name to ucprelayboard and rebased.

@hakan42
Copy link
Contributor

hakan42 commented Dec 2, 2015

  • code looks good
  • local build is successful
  • merging now 😄

hakan42 added a commit that referenced this pull request Dec 2, 2015
@hakan42 hakan42 merged commit 26bc776 into openhab:master Dec 2, 2015
@hakan42
Copy link
Contributor

hakan42 commented Dec 2, 2015

@rmichalak , Thank you very much for your contribution. Could you please test this binding after tonights Jenkins build? If successful, an announcement posting on the forums would be nice 😄

@teichsta
Copy link
Member

teichsta commented Dec 2, 2015

and please test your binding against the OH2 runtime (@hakan42 another requirement for new bindings :-))

@rmichalak rmichalak deleted the relayboard-binding branch December 5, 2015 14:53
@rmichalak
Copy link
Contributor Author

Verified on both OH and OH2 runtime. Created additional pull request #3509 to fix dependency and CRC calculation problem.

@hakan42
Copy link
Contributor

hakan42 commented Dec 5, 2015

Thank you for the confirmation. I will prepare a PR against OH2 later tonight.

@teichsta
Copy link
Member

teichsta commented Jan 7, 2016

@rmichalak after all could you please provide the according Wiki page until the weekend? Thanks, Thomas E.-E.

@rmichalak
Copy link
Contributor Author

Sure, I will try to do this before this weekend.

@rmichalak
Copy link
Contributor Author

@watou
Copy link
Contributor

watou commented Jan 8, 2016

Looks great! Very minor comment: Since inverted=true is optional, would the syntax explanation be a little better like:

{ucprelayboard="board=<name>;relay=<number>[;inverted=true]"}

(the second semicolon is only needed if you are specifying a third value)

@rmichalak
Copy link
Contributor Author

Good spot. Fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants