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

Initial contribution of the HDanywhere 2.0 binding #23

Merged
merged 1 commit into from
May 14, 2015

Conversation

kgoderis
Copy link
Contributor

Signed-off-by: Karel Goderis [email protected] (github: kgoderis)

@kaikreuzer kaikreuzer added the new binding If someone has started to work on a binding. For a new binding PR. label Dec 22, 2014
@kaikreuzer kaikreuzer self-assigned this Jan 13, 2015
@kaikreuzer kaikreuzer modified the milestone: 2.0.0 alpha2 Apr 4, 2015
xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">

<!-- HDanywhere Thing Type -->
<thing-type id="matrix">
Copy link
Member

Choose a reason for hiding this comment

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

How many variants are there? Would it be a lot of effort to define the different products directly? You define 8 channels, but some devices have only 4 of them, right? So this feels a bit awkward.
Also, if the number of input ports is known, it could be defined in the state description in order to list the valid options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Valid point, but 8x8 is the maximum they have for now. It did it that way because part of their product line is sold as bare racks + modules you can insert, so one could end up with any combination of ports on the ingress or egress of the device

@kaikreuzer
Copy link
Member

Hi Karel,

Thanks, looks good! After incorporating my comments, could you please

  • apply the code formatter
  • squash your commits into one?

Thanks!

@kaikreuzer kaikreuzer assigned kgoderis and unassigned kaikreuzer May 3, 2015
@kgoderis
Copy link
Contributor Author

kgoderis commented May 4, 2015

Ok. Maybe you wanna want to wait with the other bindings until after I have modified some of them cfr newest API changes etc?

On 03 May 2015, at 22:07, Kai Kreuzer [email protected] wrote:

Hi Karel,

Thanks, looks good! After incorporating my comments, could you please

apply the code formatter
squash your commits into one?
Thanks!


Reply to this email directly or view it on GitHub #23 (comment).

@kaikreuzer
Copy link
Member

Ok. Maybe you wanna want to wait with the other bindings until after I have modified some of them cfr newest API changes etc?

Yes, I can do that.

@buildhive
Copy link

openhab » openhab2 #77 SUCCESS
This pull request looks good
(what's this?)

@buildhive
Copy link

openhab » openhab2 #78 SUCCESS
This pull request looks good
(what's this?)

@kgoderis
Copy link
Contributor Author

kgoderis commented May 4, 2015

Ok - Should be inline with your comments, except for the first one.
K

@buildhive
Copy link

openhab » openhab2 #79 SUCCESS
This pull request looks good
(what's this?)

<!-- Port Channel Type -->
<channel-type id="port">
<item-type>Number</item-type>
<label>HDanywhere Matrix Output Port</label>
Copy link
Member

Choose a reason for hiding this comment

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

Hm, did I miss to ask for a shorter label text on this one? Like "Output Port"?


if (thingUID == null) {
thingUID = new ThingUID(thingTypeUID,
ipAddress.replaceAll(".", "_"));
Copy link
Member

Choose a reason for hiding this comment

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

This might not be enough - as mentioned in my previous comment, an IPv6 address contains colons as well. And users might enter a hostname with special characters, who knows...

@kaikreuzer
Copy link
Member

Ok - Should be inline with your comments, except for the first one.

Almost, but not quite :-)

@kgoderis
Copy link
Contributor Author

kgoderis commented May 5, 2015

Ok - did not see your additional comments until now.

Btw, I know you want to have debug lines removed, but I would suggest to keep them in until we are out of the Alpha releases. With changing API's one does need the debugging info.

@buildhive
Copy link

openhab » openhab2 #88 SUCCESS
This pull request looks good
(what's this?)

@kaikreuzer
Copy link
Member

Btw, I know you want to have debug lines removed

I think I only suggested to remove those which log about changing the thing status. They are indeed not needed anymore, because the framework now itself logs every Thing status update as a debug log message.

@kaikreuzer kaikreuzer assigned kaikreuzer and unassigned kgoderis May 12, 2015
@kaikreuzer
Copy link
Member

@kgoderis short question: Do you actually have the right to change the assignee of an issue? If so, it would be great if you do so after incorporating review comments. But as nobody seems to do it, I guess Github does not let you do it?

@kaikreuzer
Copy link
Member

Thanks Karel, I'm ready to merge - could you please squash the two commits?

@kgoderis
Copy link
Contributor Author

No, I do not have the privilege to do that, nor can I set labels etc.

@kaikreuzer
Copy link
Member

ok, that explains why nobody is doing it...

Signed-off-by: Karel Goderis <[email protected]> (github: kgoderis)
@kgoderis
Copy link
Contributor Author

done.

@buildhive
Copy link

openhab » openhab2 #124 SUCCESS
This pull request looks good
(what's this?)

kaikreuzer added a commit that referenced this pull request May 14, 2015
Initial contribution of the HDanywhere 2.0 binding
@kaikreuzer kaikreuzer merged commit 60a5d8c into openhab:master May 14, 2015
@kaikreuzer
Copy link
Member

@kgoderis: May I ask you to create a short documentation for this binding until Sunday (May 24th)?
I have created a short guide on how to do it: https://github.com/eclipse/smarthome/blob/master/docs/sources/development/extensions/bindings/docs.md
As an example how the result could look like, please see https://github.com/eclipse/smarthome/blob/master/addons/binding/org.eclipse.smarthome.binding.hue/README.md.
Thanks!

@kaikreuzer
Copy link
Member

@kgoderis: Just noticed that the README.md is still missing - would you manage to add a very short one asap? Would be good to have at least something; everything is better than a 404 link ;-)

@kgoderis kgoderis deleted the hdanywhere branch May 29, 2015 17:22
cvanorman pushed a commit to cvanorman/openhab2-addons that referenced this pull request Dec 29, 2016
Minor bugfixing and little refactoring
magx2 added a commit to SUPLA/openhab2-addons that referenced this pull request Jun 16, 2019
magx2 added a commit to SUPLA/openhab2-addons that referenced this pull request Jun 16, 2019
Flole998 pushed a commit to Flole998/openhab-addons that referenced this pull request Dec 30, 2021
jlaur pushed a commit that referenced this pull request Jan 14, 2024
* [yamahareceiver] Fix ChannelTypeProvider
* [yamaha] Fix remaining ChannelTypeProvider (#23)

Also-by: Florian Hotze <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Jan 27, 2024
* [yamahareceiver] Fix ChannelTypeProvider
* [yamaha] Fix remaining ChannelTypeProvider (openhab#23)

Also-by: Florian Hotze <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Andras Uhrin <[email protected]>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* [yamahareceiver] Fix ChannelTypeProvider
* [yamaha] Fix remaining ChannelTypeProvider (openhab#23)

Also-by: Florian Hotze <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
* [yamahareceiver] Fix ChannelTypeProvider
* [yamaha] Fix remaining ChannelTypeProvider (openhab#23)

Also-by: Florian Hotze <[email protected]>
Signed-off-by: Jan N. Klug <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants