Harman Luxury Change from REST HTTP API to WebSckets

[alon-tchelet]

This PR is fixing pretty much all the issues previously experienced with the driver as the vast majority of issues came from random timeouts and the lack of lua sockets usage.

By moving all the communication between the device and the hub to websockets we now have a stable bidirectional connection based on lua sockets. The old inefficient and blocking REST API polling was removed and the device is now able to give live updates without being polled by the hub. This results in a much more relaxed and quieter network activity by the device and hub and a smoother user experience.

Note: This PR is replacing previous improvement attempt in now closed PR #1360

[github-actions[bot]]

Invitation URL: https://bestow-regional.api.smartthings.com/invite/OzMg0o1AgD29

[github-actions[bot]]

Test Results

   60 files    377 suites   0s :stopwatch: 1 821 tests 1 821 :white_check_mark: 0 :zzz: 0 :x: 3 159 runs  3 159 :white_check_mark: 0 :zzz: 0 :x:

Results for commit a3dc9a6e.

:recycle: This comment has been updated with latest results.

[github-actions[bot]]

Minimum allowed coverage is 90%

Generated by :monkey: cobertura-action against a3dc9a6efaf2369568666f1b37c61dc02ce7d4ce

[dljsjr]

I would also advocate for using the cosock/select based lustre instead of the callback based one. As Carter mentioned, it's included in the Lua Libs, so you'd no longer need to vendor it, but you will need to adjust the API usage.

(sonos packages its own lustre but it is almost identical to what is in the lua libs with only a small change to the constructor api iirc)

As Carter mentioned, the patch to lustre in the Sonos driver is a minor tweak to the constructors. For Sonos, we need to be able to pass in our own TCP socket instead of having lustre create one implicitly. This is because the default lustre package doesn't work with wss:// out of the box, but the only thing needed for wss:// is passing in a TLS-wrapped lua socket. So the Sonos version of lustre modifies the constructor to allow for passing in our own socket. It's on our roadmap to upstream this patch so that it's available to all users, but there are some additional refactors needed to make the changes more idiomatic and ergonomic.

If that's too large of a refactor for your timelines on this PR, it's not a blocker. But it is something I would recommend you keep on your radar, in the interest of keeping the code size and memory usage for the driver at a minimum by using as much shared code as possible. It also makes sure that you don't have to back port bug fixes yourself if we improve the code in the lua libs.

[alon-tchelet]

The usage of websockets is really good for this driver, and I think it is worthwhile to pursue it. Unfortunately the lustre library used here is out of date; I assume you copied from bose which is also out of date and we intend to update some day. You should instead use the version that is made available in the lua libraries so that you are using the most recent version which has several bug fixes, and so you dont have to include any of the lustre files in the driver package. If you want an example of using the newer api, which isn't callback based, sonos is really the only example (sonos packages its own lustre but it is almost identical to what is in the lua libs with only a small change to the constructor api iirc).

I have indeed copied the Lutsre library from Bose, as I wrote in the respective commit message. But I'll take your advice and use the version from the latest library (lib v9_52x)

Sonos has a Router which is a single cosock task that manages all sending and receiving for all the websockets used in the driver. You could follow that approach or another option is to have a single task for managing each websocket individually.

Thanks for the directions

[alon-tchelet]

Hi @dljsjr and @cjswedes , I pushed the reworked version. Could you have a look? I do have an issue with the keep_alive config of the websocket. I obviously need it to inspect if the device is still online, but it seems that when I provide a number value I get this error:

FATAL Harman Luxury  runtime error: [string "cosock.lua"]:313: [string "lustre/ws.lua"]:299: attempt to index a nil value (local 'recv')
stack traceback:
    [string "lustre/ws.lua"]:299: in method '_handle_recvs'
    [string "lustre/ws.lua"]:286: in method '_receive_loop'
    [string "lustre/ws.lua"]:178: in function <[string "lustre/ws.lua"]:177>
stack traceback:
    [C]: in function 'error'
    [string "cosock.lua"]:313: in field 'run'
    [string "st/driver.lua"]:1016: in method 'run'
    [string "init.lua"]:207: in main chunk

I don't understand what I'm missing that causes it to try to access a nil value.

PS, I will clean up the commit messages when the changes are accepted

[alon-tchelet]

Cleaned the commits. I will update when the self verification is complete

[cjswedes]

@alon-tchelet we'd like to get this merged by 11am CST 7/2. Can you squash your commits down to remove the fixup commits?

edit: we will squash and merge then if you haven't.

[alon-tchelet]

Sorry, I did it locally and forgot pushing to remote. All good now. Do you know how long will it be until this driver will get to end users?

[cjswedes]

If all goes well with the release. Itll go to the beta channel today, and the production channel next week Monday.