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

[hueemulation] Update README.md #4282

Merged
merged 2 commits into from
Jan 31, 2019
Merged

[hueemulation] Update README.md #4282

merged 2 commits into from
Jan 31, 2019

Conversation

pmknowles
Copy link

It's not clear where the tag needs to be applied

It's not clear where the tag needs to be applied
@wborn wborn changed the title Update README.md [hueemulation] Update README.md Nov 26, 2018
@cweitkamp
Copy link
Contributor

Isn't the syntax clearly described in the docs? I'd vote for not putting it into the documentation of a binding.

@pmknowles
Copy link
Author

So I went to the docs

Tagging is a new feature and only a few I/O add-ons have implemented it. The easiest way to determine if tags have been implemented in a specific add-on is to see if the add-on documentation explicitly discusses their usage. Tags will be ignored if no Items in the openHAB installation support it.

See the Hue Emulation or HomeKit Add-on documentation for more details.

The link in the docs to Hue Emulation and the HomeKit gives 404 which doesn't help. My point was that the example doesn't show where to put the tag. You may be able to remember the syntax but I am not constantly adding items.
One of the frustrations of openHAB is the examples are often written by people who understand the systems really well. Something that is blindingly obvious to them isn't necessarily obvious to someone less experienced.
I'm suggesting an improvement to the example not rewriting the readme

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

Alright, I got your point. Let's add the information in a short version.

I fixed the links in the docs (see openhab/openhab-docs#819). After a new website deployment the will work again.

Copy link
Member

@davidgraeff davidgraeff left a comment

Choose a reason for hiding this comment

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

Why not changing one of the existing example items with the explanation sentence. People will figure this out.

@pmknowles
Copy link
Author

pmknowles commented Dec 3, 2018 via email

@cweitkamp
Copy link
Contributor

@pmknowles Your help is very much appreciated. I like the proposal of @davidgraeff. Wdyt?

I think openHAB could look for volunteers to review documentation prior to publication. I, for one, would be happy to help with that.

Yes, of course. We are always looking for help. Please have a look into the docs repo or contact @Confectrician directly for more information.

@davidgraeff
Copy link
Member

In the long run hopefully the majority can just use Paper UI for assigning tags etc.
My suggestion is to change one of the given examples, maybe with a shorter configuration than mqtt_1. A short config would be a typical OH2 addon config where just the destination "channel" is given.

@cweitkamp
Copy link
Contributor

@pmknowles Any news on this?

@cweitkamp cweitkamp added the awaiting feedback Awaiting feedback from the pull request author label Dec 21, 2018
Signed-off-by: Wouter Born <[email protected]>
@pmknowles pmknowles requested a review from digitaldan as a code owner January 30, 2019 17:19
@wborn wborn removed the awaiting feedback Awaiting feedback from the pull request author label Jan 30, 2019
@wborn
Copy link
Member

wborn commented Jan 30, 2019

I've addressed the review comments @cweitkamp!

Copy link
Contributor

@Confectrician Confectrician left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@davidgraeff wdyt? Shall we now leave it like it is or shall we incorporate your suggestions too?

@davidgraeff
Copy link
Member

There are all kind of examples added to the binding docs at the moment, I see this in my bindings at least.
This is not scale-able nor does it make sense if we indeed replace the .things/.items file syntax at some point. But for now I just approve all of those additions.

@cweitkamp cweitkamp merged commit 4fafd78 into openhab:master Jan 31, 2019
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Jun 19, 2019
Signed-off-by: Wouter Born <[email protected]>
Signed-off-by: Pshatsillo <[email protected]>
ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019
Signed-off-by: Wouter Born <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
@wborn wborn added this to the 2.5 milestone Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants