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

Extensive Refactoring #175

Merged
merged 94 commits into from
Mar 5, 2021
Merged

Extensive Refactoring #175

merged 94 commits into from
Mar 5, 2021

Conversation

michikrug
Copy link
Collaborator

@michikrug michikrug commented Oct 6, 2020

First of all, sorry for that (probably) not reviewable PR in size.

I did an extensive refactoring of nearly all components.

The most important changes are:


If you struggle with approving this. Please let me know and we might find a way to slice this into smaller PRs.
But with everything tested now, I doubt that there will be any issues.

Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
Signed-off-by: Michael Krug <[email protected]>
@michikrug michikrug reopened this Feb 25, 2021
@michikrug
Copy link
Collaborator Author

@marziman I also worked on a workflow to automatically deploy to Google Cloud Functions on a pushed tag: michikrug#9

@marziman
Copy link
Collaborator

Ok lets speak about that too.
How about Tuesday 09:00 pm?

@michikrug
Copy link
Collaborator Author

Ok lets speak about that too.
How about Tuesday 09:00 pm?

@marziman let's do it: https://meet.google.com/ptn-iavo-gzr

Signed-off-by: Michael Krug <[email protected]>
@michikrug
Copy link
Collaborator Author

We definitely need to establish a better communication culture (or channel) here. Again, more than an hour "wasted" without notice. This kinda sucks. 😔

@marziman
Copy link
Collaborator

marziman commented Mar 2, 2021

Hi @michikrug I am sorry, missed cause bringing kids to bed.
Maybe I will send you my nr or are you on slack?

@michikrug
Copy link
Collaborator Author

@marziman you may send me some contact details within the openhab community as a message to not expose too much personal data here

@marziman
Copy link
Collaborator

marziman commented Mar 2, 2021

Already done. You have a message.
If I would have seen meesage early I eould not let you wait. Sorry again! 🙏🏻

@RaimondB
Copy link

RaimondB commented Mar 5, 2021

I’d like to perform some test with these new features as I have motion sensors, lux, temperature and humidity sensors in multiple rooms that I would like to expose to google assistant. The first use case would be to use the motion detection to determine home or away status.

currently I am running the official version of openhab in docker. Can someone point me in the right direction to the steps I need to take to have these features available in my own version of openhab?

@michikrug
Copy link
Collaborator Author

I’d like to perform some test with these new features as I have motion sensors, lux, temperature and humidity sensors in multiple rooms that I would like to expose to google assistant. The first use case would be to use the motion detection to determine home or away status.

currently I am running the official version of openhab in docker. Can someone point me in the right direction to the steps I need to take to have these features available in my own version of openhab?

Hello. Since this integration is not part of the openHAB package itself, you would need to follow the instructions as stated in the README and set up an own version of openHAB cloud and this action.

@marziman
Copy link
Collaborator

marziman commented Mar 5, 2021

Merging after video call...

@marziman marziman merged commit 74ea9a3 into openhab:main Mar 5, 2021
@Ape
Copy link

Ape commented Mar 10, 2021

@michikrug Should these new features already work on myopenhab.org since this has been merged?

@michikrug
Copy link
Collaborator Author

@michikrug Should these new features already work on myopenhab.org since this has been merged?

Negative. There will be an announcement by @marziman in the next days.

@nonamekl47
Copy link

Hello everyone, just a quick question as it seems that I'm missing something. Is this going to be available in openhab ? Or is it only for self hosted instances?

@michikrug
Copy link
Collaborator Author

As mentioned before this is not yet deployed and publicly available.

@nonamekl47
Copy link

@michikrug thanks for the clarification, that was my initial understanding but wasn't sure. Looking forward to seeing it go live.

@eikowagenknecht
Copy link
Contributor

I see that you deliberately chose to keep the tags since you added some sentences in the documentation in that regard. I really think we should use this major refactoring and get rid of that compatibilty, it is confusing (GA binding reacts to a tag "homekit:TargetHeatingCoolingMode" for example), leads to errors (as you note in the docs) and the new way has been around for quite some time now. When will be a better time to get rid of this than now? :-)

Also default.js still has "hwVersion" and "swVersion" on "2.5.0". I think this deserves a bump to "3.0" ;-)

@michikrug
Copy link
Collaborator Author

@eikowagenknecht I discussed that with Marziman and we definitely want to get rid of tags. There is already an announcement in the openHAB community where we want to give the users a bit of time to adjust their configuration. A PR to remove the stuff is also open #194.

I think I touched the version also in some other PR...

@eikowagenknecht
Copy link
Contributor

Ah sorry, didn't see that that was already taken care of :-)

One more thing I stumbled upon when I synced the code up with the metadata UI:

You added the parameters "discreteOnlyOpenClose" and "queryOnlyOpenClose". Wouldn't it make sense to generalize this to "discreteOnly" and "queryOnly" since it can also be used in other GA Traits (that are currently not yet implemented, https://developers.google.com/assistant/smarthome/traits/energystorage for example)?

@michikrug
Copy link
Collaborator Author

Ah sorry, didn't see that that was already taken care of :-)

One more thing I stumbled upon when I synced the code up with the metadata UI:

You added the parameters "discreteOnlyOpenClose" and "queryOnlyOpenClose". Wouldn't it make sense to generalize this to "discreteOnly" and "queryOnly" since it can also be used in other GA Traits (that are currently not yet implemented, developers.google.com/assistant/smarthome/traits/energystorage for example)?

Sounds reasonable, will just do it

@eikowagenknecht
Copy link
Contributor

Great, I also added / changed this in my PR for the metadata UI.

@JohannesPtaszyk
Copy link

Is this PR already deployed?

@michikrug
Copy link
Collaborator Author

Is this PR already deployed?

No

@Ape
Copy link

Ape commented May 30, 2021

Looks like this has now been deployed.

@michikrug
Copy link
Collaborator Author

Yes. There was already an announcement in the official openHAB community. https://community.openhab.org/t/openhab-google-assistant-new-release-with-updated-device-support/121143/2

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

Successfully merging this pull request may close these issues.

Support color temperature for non-RGB lights