Harman-Luxury Improve device update communications

[alon-tchelet]

Reduced the thread responsible to poll updates from the device from one per device to a single one for the whole driver. This will help with stability when a user has a lot of devices (Harman Luxury devices in particular) connected to their hub. Moreover, the HTTP timeout time was reduced and a timeout error will now has its own error message.

[github-actions[bot]]

Channel deleted.

[github-actions[bot]]

Test Results

   57 files    365 suites   0s :stopwatch: 1 773 tests 1 773 :white_check_mark: 0 :zzz: 0 :x: 3 074 runs  3 074 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 30c2fb3e.

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

[github-actions[bot]]

Minimum allowed coverage is 90%

Generated by :monkey: cobertura-action against 30c2fb3ef50115e9fd83767738ff5f3a6bf54513

[alon-tchelet]

I agree with your comments. What we previously used was closer to what you described, but it seems the hubs or a different component in the SmartThings arrangement was unable to enable multiple driver:call_on_schedule objects (one per device) and it would create a lot of issues.

We currently don't have the infrastructure to set up cosock communication, but ideally we would like to just update SmartThings regularly as values change triggered by the device. Samsung have recommended previously to use SSE (which we also don't currently support). What is, in your opinion, the best way alternative for polling?

[cjswedes]

I agree with your comments. What we previously used was closer to what you described, but it seems the hubs or a different component in the SmartThings arrangement was unable to enable multiple driver:call_on_schedule objects (one per device) and it would create a lot of issues.

I'm a little curious about this. I know multiple call_on_schedule functions can be setup, this has been done in other drivers. Maybe the same issues I am reference wrt to many polls happening to the same device due to the function being triggered faster than it can finish its network request. Either way, using call_on_schedule per device isn't ideal anyway.

We currently don't have the infrastructure to set up cosock communication, but ideally we would like to just update SmartThings regularly as values change triggered by the device

Cosock is just the library we use for executing our socket operations on coroutines in lua. I will provide an example of how this can be done to setup your polling loops.

[cjswedes]

At the end of the device_init handler you can setup a cosock task to do the devices polling operation at the correct interval like this:

  local cancel_tx, rx = cosock.channel.new()
  device:set_field("polling_cancel", cancel_tx)
  cosock.spawn(function()
    rx:set_timeout(const.UPDATE_INTERVAL)
    while true do
      local cancelled, err = rx:receive() --Timeout is the 1 sec delay
      if cancelled or err ~= "timeout" or device == nil then -- defensive check for device being nil due to removal
        return
      end
      check_for_updates(device)
    end
  end, string.format("%s polling task", device.id))

And then when the device is removed you need to cancel that task by sending on the cancel channel, so in the device_removed handler you would add this:

  local cancel_tx = device:get_field("polling_cancel")
  if cancel_tx then
    cancel_tx:send("cancel") --anything non-nil should work
  end

I dont think there should be any errors besides "timeout" (the only other one is closed which happens when the tx side is garbage collected), so the loop should keep going as long as the driver is running. But if you were ever worried that there would be a situation where the polling loop exits and needs to be restarted, you would have to restart the polling task; you could be defensive and give the user a chance to correct this without a driver restart by cancelling and re-spawning the polling task any time the device's refresh capability handler is called.

[alon-tchelet]

I'm a little curious about this. I know multiple call_on_schedule functions can be setup, this has been done in other drivers. Maybe the same issues I am reference wrt to many polls happening to the same device due to the function being triggered faster than it can finish its network request. Either way, using call_on_schedule per device isn't ideal anyway.

Ah yes. That's possible. Perhaps I should reduce the timeout down to 1s. That's generally sufficient, but will certainly have dropped values in weaker connections.

Regarding your example, I will try to implement it and update the commits according.

[alon-tchelet]

This PR is being closed as a proper fixup is done in PR #1433