search

andrei-tatar / node-red-contrib-nora

Node Red Google Home integration
74 stars 25 forks source link

How to contribute? (given nora-common and node-red-contrib-nora are separated) #63

Open readeral opened 4 years ago

readeral commented 4 years ago

Hi @andrei-tatar,

This a great project! One thing I'm pretty keen on is being able to ask google the status of sensors I have around the place.

Now that google has released the new type 'Sensor' and associated traits, I've been working on implementing them to add to this library. (Given I'm not all that familiar with typescript, wasn't at all familiar with RxJS, and has been a few years since I've written a NodeRed node, it's been a bit rough 😅)

So - I now have a collection of files, some of which belong in this repo, some of which belong in nora-common, and I'm not entirely sure how I ought to go about using those repos together to test locally on my own branch, nor the best way to create a pull request for both libraries.

Can you give me some pointers on the best way forward?

Cheers!

Al

readeral commented 4 years ago

(this picking up on #52 )

andrei-tatar commented 4 years ago

Hi,

It's pretty hard to test locally as you need to expose https endpoints for google to talk with. There is some progress on documenting how to setup NORA in your own heroku dyno env in the README here: https://github.com/andrei-tatar/nora-service.

nora-service - is the backend service. This is what google talks with. This executes the commands and queries from google and triggers syncs. nora-common - this contains the models for the supported devices and states. It is extracted to a separate repo because it's used by both the node-red plugin and the service. It will also contain all the command handlers at some point in order to facilitate local execution path. node-red-contrib-nora - this contains the node-red plugin (the actual nodes) and the logic to communicate with the nora service.

So you probably need some files in all the repos.

Cheers

bertreb commented 4 years ago

Hi, Is it an option to extend nora-service and nora-common first with the sensor type. That way the sensor extension for node-red-contrib-nora and my plugin can be tested. If you (@andrei-tatar) agree, i assume the nora-service and nora-common changes can be proposed via PR's?

leongwaikay commented 4 years ago

Currently Google Home exposes device traits. Is it possible to allow user to create a custom device by composing traits and types together? That whay Nora need not try to implement all possible devices but just allow users to create as needed.

readeral commented 4 years ago

Sorry I’ve been sitting on this - intend to do more work on it eventually but been a bit tied up! I should commit a PR as a draft just for people to look at at least... will do so today

On Wed, 20 May 2020 at 3:42 pm, Wai Kay notifications@github.com wrote:

Currently Google Home exposes device traits. Is it possible to allow user to create a custom device by composing traits and types together? That whay Nora need not try to implement all possible devices but just allow users to create as needed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/andrei-tatar/node-red-contrib-nora/issues/63#issuecomment-631249413, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADZK3CEWKRDNSDSDYZAKGSTRSNUTLANCNFSM4MW4HXHA .

--

Alan Reader| e. areader0@gmail.com areader0@gmail.com | m. 0400 215 751 | fb. - readeral http://www.facebook.com/readeral | i. - instagram.com/readeral http://instagram.com/readeral

bertreb commented 4 years ago

@readeral, hi did you manage to work on the PR for the sensors?

readeral commented 4 years ago

Just did the PR now - but it's not a working branch by a long shot.

leongwaikay commented 4 years ago

Actually the way this is structured feels very difficult to add new nodes.

Google Home devices are just composed of traits. Same devices might be composed of different traits.

I am working to reimplement the device/state as traits, and have each device/node customize its own device type and traits. Of course we still keep simple basic device nodes like what we have now. But allow it to be customized by users.

readeral commented 4 years ago

So this is the approach I’ve taken (traits) in my PR

On Fri, 29 May 2020 at 12:16 am, Wai Kay notifications@github.com wrote:

Actually the way this is structured feels very difficult to add new nodes.

Google Home devices are just composed of traits. Same devices might be composed of different traits.

I am working to reimplement the device/state as traits, and have each device/node customize its own device type and traits. Of course we still keep simple basic device nodes like what we have now. But allow it to be customized by users.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/andrei-tatar/node-red-contrib-nora/issues/63#issuecomment-635377241, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADZK3CGJQ3SUEJXOZDHGH7TRTZW2HANCNFSM4MW4HXHA .

--

Alan Reader| e. areader0@gmail.com areader0@gmail.com | m. 0400 215 751 | fb. - readeral http://www.facebook.com/readeral | i. - instagram.com/readeral http://instagram.com/readeral

leongwaikay commented 4 years ago

So this is the approach I’ve taken (traits) in my PR

I had looked at your PR but this is not what I am talking about. Your available "traits" are actually just supported sensors under the SensorState trait. So it is still just adding a single Trait to nora.

Take a look at https://developers.google.com/assistant/smarthome/traits and you will see about 30+ traits that Google Home supports. Each device type can support any composition of traits.

My proposal is to code nora in a way to support all devices and traits so any user can customise their own device in nora so they do not need to depend on writing code to customise their device.

bertreb commented 4 years ago

@leongwaikay your approach makes Nora more flexible. I like that because i'm using only the backend and with that flexible approach i could introduce easier new devices/sensors. What about the mapping toward Google Assistant and getting relevant voice control and info, when you just can mix any device/traits?

leongwaikay commented 4 years ago

What I understand from the Google developers documentation, all attributes, including voice control are based on the traits. I have not looked into detail to see if there are any contradicting traits but so far it seems not.

I can easily refactor the backend (nora-service) and the models (nora-common) but I'm afraid I am very new to node-red and the frontend UI (node-red-contrib-nora). One of the issues you can imagine is how to code a dynamic node configuration UI into node-red. Anybody with experience with this can help?

bertreb commented 4 years ago

The backend changes you're proposing would be very helpfull, that way i can easily support all the devices used in my Pimatic plugin. For the node-red-contrib-nora changes i can't really help. Are you preparing a PR and/or having contact with @andrei-tatar about this?

leongwaikay commented 4 years ago

For now I am getting myself familiar with the code by adding in a fan and aircon device which I require for my own use. Once successful then I will proceed to refactor the code. But the frontend has to also be refactored since it shares the common models with the backend.

I have forked the three repositories which you can follow and when done will prepare a PR. Still some way to go before the complete refactoring.

bertreb commented 4 years ago

Thanks and succes with the changes.

leongwaikay commented 4 years ago

my Pimatic plugin.

Could you share how your plugin uses the Nora backend? If I can't figure out Nora's reactive architecture maybe I'll just port over to Pimatic.

bertreb commented 4 years ago

Sure, if you can read coffeescript here is the repos of my pimatic-assistant plugin. The main file is pimatic-assistant.coffee and per device type an adapter is doing the specific config and actions. I also use a kind of smart mapping of Pimatic devices towards Nora/GA. To get a temperature sensor is use a thermostat and keep the setpoint equal to the ambiant temperature. That way you can ask what is the temperature ... . Real sensor support would be better.

leongwaikay commented 4 years ago

I've reached a roadblock.

Got this warning after deploying on Heroku: [warn] nora: sync error (schema is invalid: data.additionalProperties.anyOf[0].properties['availableFanSpeeds'].additionalItems.anyOf should NOT have fewer than 1 items)

Not sure how to progress from here

bertreb commented 4 years ago

Are you using the 'api' to the backend (nora-service/common) or something else?

leongwaikay commented 4 years ago

Yes I am using whatever is being used here. You can check out my forks of nora-service, nora-common and node-red-contrib-nora on the fan-aircon branch.

Initially I set the default value of availableFanSpeeds to empty list/array, which seems to be what the message is saying. So I added a default speed and verified that the sync.json in nora-service shows the default content. Actually, I'm not even sure how the schema is even used.

Btw, is this even the right place to be discussing this?

bertreb commented 4 years ago

So, you are deploying your own Nora on Heroku for testing? What other place would you use. @andrei-tatar should be able to answer the questions, but i think its depending on your goal.

andrei-tatar commented 4 years ago

The schema is used to validate the sync/update objects that come from node-red. It's easier to make sure no garbage gets sent and the state is clean.

The one in the repo might not be up to date but it's generated on build based on the typescript interfaces.

leongwaikay commented 4 years ago

Yes I deployed on Heroku. I'm having issues with the sync object validation. Need to figure out why the device attributes aren't sent in the sync.

Now trying the Heroku remote debugging ssh tunnel

leongwaikay commented 4 years ago

@andrei-tatar one of the properties in my fan device is a custom array containing arrays of strings, e.g. [['low', 'speed 1', 'minimum'], ['medium', 'speed 2'], ['high', 'speed 3', 'max', 'maximum']]

It's user definable so it is not fixed. How to get the typescript-json-schema to generate a general schema that matches any such format? I think that is causing the invalid state.

Edit: ah I found my answer. I need to declare it using string[][].

realjax commented 4 years ago

What I understand from the Google developers documentation, all attributes, including voice control are based on the traits. I have not looked into detail to see if there are any contradicting traits but so far it seems not.

I can easily refactor the backend (nora-service) and the models (nora-common) but I'm afraid I am very new to node-red and the frontend UI (node-red-contrib-nora). One of the issues you can imagine is how to code a dynamic node configuration UI into node-red. Anybody with experience with this can help?

If needed I can help with Node-red UI matters. I too think that Nora in its current state is on a dead end track. Please fork your changes into a new repo and then take it from there.

leongwaikay commented 4 years ago

I have already forked the repos on my github and got fan and aircon incorporated. Next step is to refactor into traits so we can have a custom device node. We can discuss further on my fork if you wish.