Performance for multiple attributes (brightness, colour loop & colour) for multiple lamps is a bit slow about hueplusplus HOT 17 CLOSED

Hi, very nice that you find the library helpful.

The delay between messages is by design, because there are limitations in the ZigBee communication.
If you send too many messages too quickly, the connection is dropped by the bridge. (Some people had issues when looping through all lights and turning them all on or off without any delay)

We have something similar to the way you described already implemented on the development branch. light.transaction() allows you to send all requests in one message, so it should have less delay. It would be nice if you could check it out and give some feedback on how easy to use it is and what could be improved.

Thank you very much

from hueplusplus.

I tried to paste my code, but there was an error message I didn't notice because you can only paste picture files.
Header file is:

#pragma once
#include "HueLight.h"

class HueLightHelper : HueLight
{
private:
	nlohmann::json requestJson;
	void setUpForColorXY(float x, float y);

protected:

public:
	HueLightHelper(HueLight& huelight);
	void setupForColorLoop(bool colourLoop);
	void RemoveColorLoop();
	void setupForColorRGB(int red, int green, int blue);
	void setupForBrightness(int brightness);
	void setupForOffOn(bool on);
	bool SendPutRequests(const int transition = 4);
};

CPP code file is:
#include <string>

#include "include/HueLightHelper.h"

#include "include/HueExceptionMacro.h"
#include "include/HueLight.h"
#include "include/json/json.hpp"
#include "include/Utils.h"

HueLightHelper::HueLightHelper(HueLight& hueLight) : HueLight(hueLight)
{
    refreshState();
    requestJson = nlohmann::json::object();
}

void HueLightHelper::setupForColorLoop(bool colourLoop)
{
    std::string effect = colourLoop ? "colorloop" : "none";
    if (effect != requestJson["effect"])
    {
        requestJson["effect"] = effect;
    }
}

void HueLightHelper::RemoveColorLoop()
{
    if (requestJson.count("effect") > 0)
    {
        requestJson.erase("effect");
        assert(requestJson.count("effect") == 0);
    }
}

void HueLightHelper::setupForColorRGB(int red, int green, int blue)
{
    if ((red == 0) && (green == 0) && (blue == 0))
    {
        setupForOffOn(false);
    }
    else
    {
	    const float redf = float(red) / 255;
	    const float greenf = float(green) / 255;
	    const float bluef = float(blue) / 255;

	    // gamma correction
	    const float redCorrected = (redf > 0.04045f) ? pow((redf + 0.055f) / (1.0f + 0.055f), 2.4f) : (redf / 12.92f);
	    const float greenCorrected = (greenf > 0.04045f) ? pow((greenf + 0.055f) / (1.0f + 0.055f), 2.4f) : (greenf / 12.92f);
	    const float blueCorrected = (bluef > 0.04045f) ? pow((bluef + 0.055f) / (1.0f + 0.055f), 2.4f) : (bluef / 12.92f);

	    const float X = redCorrected * 0.664511f + greenCorrected * 0.154324f + blueCorrected * 0.162028f;
	    const float Y = redCorrected * 0.283881f + greenCorrected * 0.668433f + blueCorrected * 0.047685f;
	    const float Z = redCorrected * 0.000088f + greenCorrected * 0.072310f + blueCorrected * 0.986039f;

	    const float x = X / (X + Y + Z);
	    const float y = Y / (X + Y + Z);


	    setUpForColorXY(x, y);
    }
}

void HueLightHelper::setUpForColorXY(float x, float y)
{
    requestJson["xy"][0] = x;
    requestJson["xy"][1] = y;

    //if (std::abs(state["state"]["xy"][0].get<float>() - x) > 1E-4f
    //    || std::abs(state["state"]["xy"][1].get<float>() - y) > 1E-4f
    //    || state["state"]["colormode"] != "xy")
    //{
    //    requestJson["xy"][0] = x;
    //    requestJson["xy"][1] = y;
    //}
}

void HueLightHelper::setupForBrightness(int brightness)
{
    assert(brightness >= 0);
	
    if (brightness > 254)
    {
        brightness = 254;
    }
	
    if (requestJson["bri"] != brightness)
    {
         requestJson["bri"] = brightness;
    }
}


void HueLightHelper::setupForOffOn(bool on)
{
    if (requestJson["on"] != on)
    {
        requestJson["on"] = on;
    }
}

bool HueLightHelper::SendPutRequests(const int transition)
{
    if (requestJson.count("on") == 0 &&
        requestJson.count("effect") == 0 && 
        requestJson.count("xy") == 0 &&
        requestJson.count("bri") == 0)
    {
        // Nothing needs to be changed
        return true;
    }
    else
    {
        if (transition != 4)
        {
            // ReSharper disable StringLiteralTypo
            requestJson["transitiontime"] = transition;
            // ReSharper restore StringLiteralTypo
        }

    	//if we are switching off, the do not supply a colour loop command.
        if (requestJson.count("on") > 0 &&
            requestJson["on"] == false &&
            requestJson.count("effect") > 0)
        {
            RemoveColorLoop();
        }

    	//if we are switching on and no colour loop supplier, then switch that off.
    	if (requestJson.count("on") > 0 &&
            requestJson["on"] == true &&
            requestJson.count("effect") == 0)
    	{
            setupForColorLoop(false);
    	}

        nlohmann::json reply = SendPutRequest(requestJson, "/state", CURRENT_FILE_INFO);

	    // Check whether request was successful
	    return utils::validateReplyForLight(requestJson, reply, id);
    }
}

from hueplusplus.

Sorry, although I have used git commerically (via a GUI), I am note sure with Github how to find the development branch to get your intermediate code.

from hueplusplus.

Did you clone the repo via git or downloaded it as a zip file?
If you cloned it, you can use a git command line
git stash (to save your changes, because they conflict)
git checkout development

When you download a zip, you can select the branch on github and then download that branch.

from hueplusplus.

from hueplusplus.

The only difference between < and " is that " also searches the current file path. Because the include directory is always added by cmake, there is no difference between the two when used to access the header files from the cpp files. The rest is just personal preference.

from hueplusplus.

from hueplusplus.

When you do not use cmake, you will have to add the include directory to the include path, which you should do either way when using the library so you don't have to specify relative paths everywhere.

from hueplusplus.

Hi Jan,
Unfortunately in its current form, I was unable to use the light.transaction().
This is because (as I understand it and my C++ is a bit out of date) you have to use transaction in the form (as shown in the comments):
light.transaction().setOn(true).setBrightness(29).setColorHue(3000).setColorSaturation(128).commit();
What I really want to do is:
t = light.transaction();
If (..)
{
t.setOn(true)
}
If ( ..)
{
t.setBrightness(20)
}
t.commit();

The reason for this requirement is that I have a rules language in which you can say:
SETDEVICE Lounge.MoodLight1 OFF;
SETDEVICE Lounge.MoodLight2 OFF;
SETDEVICE Lounge.MoodLight3 ON Blue;
SETDEVICE Lounge.MoodLight4 DIM12 Red COLOURLOOP;
This gets compiled down to an automation language byte code for a virtual automation machine. Then the virtual automation machine attempts to set the lamps dynamically at runtime (when the television gets switched on). Because the runtime system may or may not be able to reach the Philips Hue hub and may or may not be able to reach the individual lamps, the runtime system maintains a wanted and an actual state for each device. Where there is a difference, it puts messages onto a queue which another thread picks up and sends messages to the hub (if it can). The second thread sends messages back to the first thread on whether it was successful.
So effectively I created a transaction by creating a JSON message. The brightness, colour, colour loop and off/on are added to the message and then the ‘commit’ part is where the message is sent and the destructor is called.
It isn’t as neat as your solution, but enables me to meet my requirement (to have pretty lights on whilst I watch television).
I hope that feedback helps you.
Regards
Richard Croxall

from hueplusplus.

Thank you for the feedback. I did not think of this use case. This can be made to work, however it then requires additional care by the user.
In the current state, it is almost impossible to keep the transaction alive for longer than the light is valid.
When you enable the user to hold the transaction as a variable, the user has to take care that the light stays alive for longer than the transaction. Would you think this trade off is worth it?

from hueplusplus.

It is for my requirement. For other people, I don't know.

from hueplusplus.

Maybe I'm biased because I am thinking only about my own application but where a person wants to directly operate a lamp, I think he will be using a phone app to directly control the lamp.
Where there is some automation software which needs to control a lamp, it will be using an interface similar to HuePlusPlus. My guess is that such an application would generally be deciding dynamically what to do with each lamp rather than statically. So I think the use case I have described should be popular.
Clearly you must have other people using HuePlusPlus besides me. What do the other users need HuePlusPlus for?

from hueplusplus.

Hi Jan, you will be pleased to hear that although I am probably not using any of your new features that after a fight with the makefile to add new files and add the "src/" folder, the code compiles and switches my 6 lights off an on again with brightness, colours and color looping.
I will test whether they come on in the dark when my television is on later this evening.
regards
Richard

from hueplusplus.

The above is on the target environment of a raspberry pi rather than Windows.

from hueplusplus.

Clearly you must have other people using HuePlusPlus besides me. What do the other users need HuePlusPlus for?

Hey Richard we actually created this library for our home automation system Home++, because we did not like the other available libraries.

from hueplusplus.

Hi Jan,
I see you put an upgrade in yesterday.
I downloaded a copy and (with a few tweaks for compilation on Windows/Raspberry Pi) it works fine.
So I have deleted my "HueLightHelper" and am now using "StateTransaction".
Thanks for enhancement. I know you were sorry to lose the protection you had carefully coded in before.
So tonight we can see whether I get colour looping lights when the television is on!
regards
Richard

from hueplusplus.

Very nice, I'm closing this issue then.

from hueplusplus.