search

Apollon77 / daikin-controller-cloud

Connect and Control Daikin Cloud devices
MIT License
96 stars 26 forks source link

Switches to Daikin's Onecta API, ports to Typescript #139

Closed jacoscaz closed 3 days ago

jacoscaz commented 4 days ago

As initially discussed in #138 , this PR refactors the DaikinCloudController and DaikinCloudDevice classes to use an OIDC client for Daikin's Onecta API that I had developed for a different project while unaware of this one.

Additionally, given a) the extent of the refactoring, b) the fact that the OIDC client I had developed was already written in Typescript and c) @Apollon77's interest in porting to Typescript, this PR converts the entire project.

Note that all payloads returned by the Onecta API are typed using any. This is intentional as strong payload typing without actual payload validation would introduce dangerous assumptions that could easily break in confusing manners. We can take care of both payload types and validation in a different PR, if needed / requested.

jacoscaz commented 4 days ago

All right @Apollon77 , I think I'm done for the day. If you'd rather take over moving forward, feel absolutely free to go ahead. Otherwise I'll wait for your review and address your feedback as soon as I can manage.

jacoscaz commented 4 days ago

@Apollon77 just saw the email notifications about the failed pipelines. Looks like the issue is that there is no pre-compiled build of Node.js 14.x for macOS on ARM64. Looking at https://nodejs.org/en/about/previous-releases we're well past the maintenance window of the 14.x branch (and, in fact, we're also past that of 16.x). I'll drop 14.x from the test targets.

Apollon77 commented 4 days ago

Hi @jacoscaz thats cool.

Also got too late here on my side. I will merge it tomorrow my time, would do some smaller adjustments (code style, use "real" # privates instead underscore notation, docs, maybe reintroduce "tokensafer" as easy way for end users for now, maybe check for the node-force tls client for certs, and make certs dir configurable, add you to package.json contributors :-)) ... and then publish an alpha to npm ... Then you can use this already and also homebridge.

I might do some stuff directly in repo and bigger changes I will do as a PR that you can also have a look before I merge it.

Apollon77 commented 4 days ago

Re testing:I will also update that and other deps then tomorrow ... also gha might want some tweaks and updates :-)

Re types: you are completely right ... most of the "any" are needed that way because of the "Non typed api" from daikin ...sooo lets see how we continue there, but all fine for now.

JeroenVdb commented 4 days ago

Great work @jacoscaz and @Apollon77 As soon as this is released I will check on how to implement this in my homebridge plugin. As far as I can tell it should be straight forward.

jacoscaz commented 3 days ago

@Apollon77 just to let you know, I started testing this branch in my Onecta - MQTT bridge and added a few tweaks for a couple of edge cases I stumbled in. I'm done now.

Apollon77 commented 3 days ago

Alright. thank you ...! Did not found time so far , was a bit chaotic on other topics. so after dinner