search

mfucci / node-matter

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

Initial commit of a bare-bones Level Control Cluster. #255

Closed JimBuzbee closed 1 year ago

JimBuzbee commented 1 year ago

Tested as an intensity adjustment cluster with Google Home and Alexa. Work still needs to be done to support more commands and to potentially interact with onOff clusters on the same endpoint.

JimBuzbee commented 1 year ago

@Apollon77 - Thanks for the comprehensive review. I think you could go blind reading through those specs ;-) Who knew that adjusting the intensity of of a light bulb could take 12 pages of descriptions and explanations!

I'll start in on your comments today.

JimBuzbee commented 1 year ago

Updates pushed

JimBuzbee commented 1 year ago

@mfucci - Thanks for the detailed review. Maybe, just maybe, I'll eventually learn Typescript :-) I did take the advice from you and @Apollon77 to start using VS instead of vi. Big step up, but I do miss vi's navigation and search/replace commands :-)

I think I've addressed all of your comments. I left the LevelControlCluysterServer in becuase neither Google Home or Alexa call any of the unimplemented functions, but I did add the recommended "Error" throw if they were to be called.

Apollon77 commented 1 year ago

@JimBuzbee if you like , join discord https://discord.gg/EJysjRsx and we can answer questions maybe more direct then via comments here ...

JimBuzbee commented 1 year ago

Tried to log into discord, but it insisted on me having an .edu address - no go :-)

Apollon77 commented 1 year ago

@JimBuzbee Look at https://github.com/mfucci/node-matter/pull/255/commits/1b40bae16cc4f7fe5e2ebc8531f88af8444c96e0 ... I did some changes how I meaned it ... I would do it that way.

JimBuzbee commented 1 year ago

@Apollon77 - Funny - when you recommended "....commands," I thought you were just using shorthand, not literal "..." Something new for me!

Apollon77 commented 1 year ago

;-) Added even another commit ... so it is even more to the specs. Will look over all again tomorrow. Too tired

Apollon77 commented 1 year ago

Ok, should I merge it then or do you have stuff still open?

JimBuzbee commented 1 year ago

@Apollon77 - Go head and merge. I have an implementation of the PulseWidthModulationLevelControlClusterHandler that is pretty close, but I have no good way to test since nothing uses that cluster.

Apollon77 commented 1 year ago

Ok, I think we will have that for other clusters too ... so best effort and then we see when someone use it