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] All Items are discovered #5847

Closed
5iver opened this issue Jul 20, 2019 · 8 comments
Closed

[hueemulation] All Items are discovered #5847

5iver opened this issue Jul 20, 2019 · 8 comments

Comments

@5iver
Copy link

5iver commented Jul 20, 2019

With previous version of hueemulation, only Items with specific tags would be discovered. Since S1585, all Items will be discovered unless they have an 'internal' tag. Adding tags for managed Items is very tedious. This could also be considered a potential security risk to have all Items exposed to Alexa, especially unknowingly.

As @davidgraeff stated in early May...

There are no tag restrictions for sensors in place that is correct. At the moment everything that looks like a sensor is exposed.

I wanted to add tags for sensors but we needed to hurry with the merge to finish the migration.

I’ll come back to it, but busy with another openhab topic rn.

IMO, this is something that should be completed before 2.5M2 is released.

@bjoernbrings
Copy link
Contributor

Having looked into the code the fix doesn't seem to be too difficult. But it seems to be a design decision having not to flag exposed items.
(Flagging is defined in ConfigStore.java and executed in StateUtils.java)

So it seems to me like everything works as designed but you are unhappy with the design.

@5iver
Copy link
Author

5iver commented Jul 21, 2019

How can it be working as designed? The hueemulation binding has always worked differently than this, it was an admitted gap in functionality that was planned to be corrected, and it is a breaking change.

@bjoernbrings
Copy link
Contributor

Excerpt from the readme:

The service tries to expose as much items as possible (greedy), based on some criteria as explained in the section above. If you want to exclude items, you need to tag them

That sounds to me like a design decision by the author of the binding.
Like said in the statement above (and you probably know better than me, given your numerous PRs 😃) a change should be quite easy if there is an agreed design how it should be (switch to include or exclude by tag or another tag to include or switch the exclude tag to include).

@5iver
Copy link
Author

5iver commented Jul 21, 2019

That sounds to me like a design decision by the author of the binding.

No. The author of the binding restricted discovery to Items with homekit tags (#654), which then evolved to Lighting, ColorLighting and Switchable tags (#946).

The text that you quoted was added along with the code that changed this functionality in #5243. <rant> It's baffling how the approving maintainers could have justified so many functional changes in a single PR that was disguised as a bnd migration. It's obvious that the PR was approved without anyone having actually tested the changes, because it no longer worked... and nearly three months later, it still doesn't! This addon caused a lot of issues due to other untested changes squeaked in before the 2.4.0 stable release, which makes it even more surprising that more care wasn't taken to ensure that it functioned. It's basically been broken since before 2.4.0, which is why I use a jar from November. The 2.5M1 version would turn on the bedroom fan when asked to turn on the family room television (known issue).

There's also the core automation functionality that was included in this addon in the same PR, which will eventually need to be removed... 🙄 </rant>

a change should be quite easy if there is an agreed design how it should be (switch to include or exclude by tag or another tag to include or switch the exclude tag to include).

I vote to revert to the previous tagging functionality so that users will not need to make any changes to their tags when upgrading the binding from 2.4.0 or 2.5M1.

@J-N-K
Copy link
Member

J-N-K commented Jul 21, 2019

There were several reports that it works before approval.

@5iver
Copy link
Author

5iver commented Jul 21, 2019

There were 4 commits after this comment where David made a change and created the v29 jar...

"Let me quickly change that and make the service accept all types of input"

... but none of the subsequent commits appear to included that change. So, the reports that it was working were not based on the code that was merged.

@davidgraeff
Copy link
Member

davidgraeff commented Jul 21, 2019

The hue emulation pre 2.4 will not work for long anymore. It's an incomplete buggy Philips Hue bridge V1 API emulation that was also super inefficient. It walked over all items each time a command was issued. So 1000 items means a worst case linear list traversal of 1000 items. Newer Google Homes do not support the V1 bridges and I guess Amazon will eventually also drop support.

I have rewritten everything, that is true. It was a V2 API compliant, complete implementation. The 4 PRs that you have found are about some Alexa Echo Firmwares that send json without the json mimetype. The very strict jax-rs router ignored those requests, so I made the code less compliant, less strict to be compatible with more Alexas.

The new code allows the Hue App to create rooms, groups, rules, and timers. Alexa does only support lights nothing more. It does not support sensors. So the sensor exposure of all sensor-like items does not affect Alexas at all.

There is one thing missing to be recognised as a real Hue bridge. And that is an SSL certificate that contains the mac address in the certificate subject. All new devices and apps are using http only to request the mac address and than switch over to https.

There's also the core automation functionality that was included in this addon in the same PR, which will eventually need to be removed... 🙄

Not by me. I don't like how core is maintained and developed, and I don't need to as an addon developer. I can always just ship all required parts myself, which I did with this service.

@hmerk
Copy link
Contributor

hmerk commented Jul 25, 2021

closed due to inactivity

@hmerk hmerk closed this as completed Jul 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants