Improving useful command rate of hueplusplus

[LiSongMWO]

Note: I'm filing this bug as general feedback on the API design that you may want to consider (or ignore, up to you). I've already hacked together a fix in my fork to make my application work. Suggested fixes could include a parameter to control refresh behaviour or a configurable staleness tolerance on the last refresh before refreshState() is allowed and possibly making refreshState() always refresh all lights through the appropriate HTTP endpoint.

Related to #64 as I was searching for ways to fix the crash I was experiencing I found that at some point I didn't get a valid response from the hub and an instance of std::system_error was thrown because the response from the bridge wasn't valid (most likely it was a 429 response because of a high amount of connections made (as an aside I'm not sure if throwing is appropriate for a response that is fully expected and not a bug, but my fork is old by now and that might have changed since then).

What was surprising to me was to find that light.refreshState() is called eventually from Ligst::setX functions, e.g. here: https://github.com/enwi/hueplusplus/blob/83d4883211e61a66867f5736a96c667652ce333e/hueplusplus/SimpleBrightnessStrategy.cpp#L34

In my usage like so:

int main() {
  auto bridge = find_bridge();

  while (true) {
    try {
      auto t = std::time(nullptr);
      auto* tm = std::localtime(&t);
      auto day_hour = tm->tm_hour + tm->tm_min / real{60};
      auto point = schedule_lerp(schedule, day_hour);

      for (auto& light_ref : bridge.getAllLights()) {
        auto& light = light_ref.get();
        if (light.isOn()) {
          light.setBrightness(c_max_brightness * point.lum);
          light.setColorTemperature(light.KelvinToMired(point.ct));
        }
      }
    } catch (const std::system_error& err){
      std::cout<<"Caught std::system_error, what(): " << err.what()<<std::endl;
    }
    std::this_thread::sleep_for(5s);
  }
}

with 6 lights I ended up calling refreshState() once in getAllLights to update the state for all lights once, then three times for each active light (in isOn() non const, setBrightness and setColorTemperature). This results in 19 calls to GET requests to the hub out of which only the first one in getAllLights was actually needed. Adding the two setX calls per enabled light, this results in a total of 31 connections to the hub in a burst every 5 seconds or 6.2 connections per second on average... When in reality all that was needed was 7 connections (1+2*n) (or 4 connections (1+n) if the API would support writing Brightness and Colour Temperature in one request)...

I have fixed this for myself in my branch by simply removing calls to refreshState() from the various Strategy classes and I'm hoping that this will improve scalability and responsiveness of my application.

[enwi]

@EmilyBjoerk Short answer is that there are already some changes on the development branch that reduce the amount of refreshing needed, which is even further reduced on shared-state branch. But @Jojo-1000 can give you a more profound answer to this.

What you could do in the meantime is try out the development branch, as it will be merged into master soon

if the API would support writing Brightness and Colour Temperature in one request)...

This is also already taken care of on the development branch where you can use transactions for updating multiple light properties -> https://github.com/enwi/hueplusplus/blob/development/src/StateTransaction.cpp

[LiSongMWO]

Thanks for the input, like I said I've already hacked it to work in my fork. I might take a look at the dev branch eventually but right now it's working for me and I need to spend less time at the computer for health reasons.

[enwi]

Then I will close this issue now since it does not seem relevant anymore due to the changes on the development branch