mihai-dinculescu / tapo

Unofficial Tapo API Client. Works with TP-Link Tapo smart devices. Tested with light bulbs (L510, L520, L530, L610, L630), light strips (L900, L920, L930), plugs (P100, P105, P110, P115, P300), hubs (H100), switches (S200B) and sensors (KE100, T100, T110, T300, T310, T315).
MIT License
313 stars 30 forks source link

Bug in hue/saturation and color_temperature code #225

Closed avadiax closed 4 weeks ago

avadiax commented 4 weeks ago
/// Sets the *hue* and *saturation*. ['ColorLightSetDeviceInfoParams::send'] must be called at the end to apply the changes.
/// The device will also be turned *on*, unless ['ColorLightSetDeviceInfoParams::off'] is called.
///
/// # Arguments
///
/// * 'hue' - between 1 and 360
/// * 'saturation' - between 1 and 100
pub fn hue_saturation(mut self, hue: u16, saturation: u8) -> Self {
    self.hue = Some(hue);
    self.saturation = Some(saturation);
    self.color_temperature = Some(0);

    self
}

/// Sets the *color temperature*. ['ColorLightSetDeviceInfoParams::send'] must be called at the end to apply the changes.
/// The device will also be turned *on*, unless ['ColorLightSetDeviceInfoParams::off'] is called.
///
/// # Arguments
///
/// * 'color_temperature' - between 2500 and 6500
pub fn color_temperature(mut self, value: u16) -> Self {
    self.hue = Some(0);
    self.saturation = Some(100);
    self.color_temperature = Some(value);

    self
}

Referring to the above code snippet in 'tapo/src/requests/set_device_info/color_light.rs' between line 65 to 92.

The code is wrong because hue must be 1 to 360, but in color_temperature and certain colors in COLOR_MAP, it is hard_coded to 0. Also, there are bulb color settings where the hue, saturation and color_temperature can co-exist such as color temperature is "3300", hue is "10", saturation is "19". Using the command:

await device.set().brightness(80).hue_saturation(10, 19).color_temperature(3300).send(device)

... will always trigger a Validation error.

await device.set_hue_saturation(hue, saturation) Exception: Validation { field: "hue", message: "must be between 1 and 360" }

Please confirm if this is the case. Correct fix is that in 'fn hue_saturation', the line 'self.color_temperature' should be removed. In 'fn color_temperature', the lines 'self.hue' and 'self.saturation' should be removed.

mihai-dinculescu commented 4 weeks ago

You're correct, hue should accept a value of 0. I've fixed this in the referenced commit.

However, I do not think the API should allow setting both color_temperature and the hue and saturation pair. Whenever color_temperature is set, hue and saturation seem to have no effect.

Getting the device info confirms this. hue and saturation will be returned as None when set together with color_temperature.

avadiax commented 4 weeks ago

Thanks! Look forward to the new update.