search

mfucci / node-matter

Matter protocol client / server for node.js
Apache License 2.0
119 stars 10 forks source link

Not ready to merge. Debugging issues #246

Closed JimBuzbee closed 1 year ago

JimBuzbee commented 1 year ago

Added Level Control Cluster to On/Of dimmer device and both Google Home and Alexa show intensity adjustment sider on light device, but my commands in my LevelControlServer never get called.

I see standard debug messages in console when slider adjusted, but no debug messages from my commands in LevelControlServer show up.

Not clear on how arguments are passed to commands in LevelControlServer

Not clear on 2nd and 3rd arguments when new ClusterServer is instantiated in Device.ts

Apollon77 commented 1 year ago

Can you post the pairing log of that please? (ideally update from Github ... merged several things today). I assume something but need to see what they do.

Because of all being optional commands it could be that we need to add support for a property where controllers can request which commands are implemented and which not ;-) We do not have that implemented till now. The debig log would let is see that. So ifg you se that it does a read request on 0xFFF9 attrbute then it is most likely this ...

A try (if it is this) would be to manually add in the same struct where you added featuremap another line with

acceptedCommandList: [new CommandId(0),new CommandId(1),new CommandId(2),new CommandId(3)]

We indeed will do this automatically latert based on the definition of methods available :-)

JimBuzbee commented 1 year ago

Here's a log of pairing. At the end I turn the light on and then try to adjust the intensity. You'll also see some of my debug logging where I attempted to see what was being passed from Google Home to node-matter.

LevelControl.txt

Apollon77 commented 1 year ago

Ok, then I need to look deeper into this. I think for now the ideal progress is to do the cluster definitions itself acording to specs and do logic a bit later. I will put it on my list to check that.

JimBuzbee commented 1 year ago

Thanks for looking. Will proceed.

I tried to update the "acceptedCommandList" as suggested, but have not been able to get it right. Alternatively, I just changed the commands from "OptionalCommand" to "Command" - no change in behavior.

Apollon77 commented 1 year ago

Yes, it is also not reading the properties I thought ,... so needs to be something else ... need to see

JimBuzbee commented 1 year ago

Made a little more progress and it looks like it is my problem. Google is calling moveToLevelWithOnOff which I didn't expect so it was commented out. handleInvokeRequest couldn't find the command so it just returned. If I uncomment, I get a stack trace of

"ERROR InteractionMessenger Error: Unexpected type 20, was expecting 4."

in CommandServer, ObjectSchema.decodeTlv...

Looks like I have something mis-defined again. Will continue on.

JimBuzbee commented 1 year ago

I have the Level Control Cluster added to the On/Off device working fairly well with both Alexa and Google home. Both show intensity adjustment controls and pass the value back to node-matter where I attached a Listener/Script for action.

Alexa calls "MoveToLevel" and Google calls "MoveToLevelWithOnOff". Screenshot attached where I have node-matter controlling the on/off and intensity of some old Govee LED Light strips. node-matter sensor devices showing cold and snowy today!

I'll clean up the code a bit and add to this Draft today, but there is still work to do. Spec calls for "WithOnOff" commands to to also modify OnOff cluster state if on same endpoint.

Open questions: As a Typescript novice, the syntax in ClusterServerHandlers is a bit beyond me. Is there a good example to follow? I think I need to be returning values to Alexa/Google Home? and I'm not sure about the input parameters to the handlers either.

image

Apollon77 commented 1 year ago

@JimBuzbee As stated above, the "Inter cluster acces" is not yet implemented, so in the current structure this is not possible to easiely do and if you would do stuff we change soon.

I work on the future "high level api structure" and one part of this is to allow clusters to access other clusters on the same endpoint, so we should have a very first idea in some days.

I would propose to "stop here" for now and continue somewhere else (or just finish the pure cluster specs and bring in as PR for LevelCluster) and we contzinue with the logic afterwards. I stumbled over this as I worked on the full logic for onoff cluster, scnes and groups and also stopped for now :-)

So give me some days and we have a basis in latest 1-2 weeks hopefully (as PR earlier)

JimBuzbee commented 1 year ago

OK - Just pushed updates. Can take PR out of draft state if it looks reasonable.

Apollon77 commented 1 year ago

@JimBuzbee maybe lets just have the pure cluster definition as own PR? and we stay here as is till new API is there

JimBuzbee commented 1 year ago

Sounds good. Will wait for a new PR until the new API is ready

Apollon77 commented 1 year ago

If you like: CLuster definition PRs are always welcome :-)) They just needs to be checked against specs :-)

JimBuzbee commented 1 year ago

Yeah, sorry - forgot that all that other test/example code was in there. Will create a new PR afterwhile.