Closed pwoerndle closed 9 months ago
I did some more digging and the EmptyResult error is thrown in control_child method in tapo/src/api_client.rs
on the last statement.
pub(crate) async fn control_child<R>(
&self,
device_id: String,
child_request: TapoRequest,
) -> Result<R, Error>
where
R: fmt::Debug + DeserializeOwned + TapoResponseExt,
{
debug!("Control child...");
let params = MultipleRequestParams::new(vec![child_request]);
let request = TapoRequest::MultipleRequest(Box::new(TapoParams::new(params)));
let params = ControlChildParams::new(device_id, request);
let request = TapoRequest::ControlChild(Box::new(TapoParams::new(params)));
let responses = self
.protocol
.execute_request::<ControlChildResult<TapoMultipleResponse<R>>>(request, true)
.await?
.ok_or_else(|| Error::Tapo(TapoResponseError::EmptyResult))?
.response_data
.result
.responses;
let response = responses
.into_iter()
.next()
.ok_or_else(|| Error::Tapo(TapoResponseError::EmptyResult))?;
validate_response(&response)?;
response
.result
.ok_or_else(|| Error::Tapo(TapoResponseError::EmptyResult))
}
Since the control_child method is used for read and write operations, I guess we need to check the result to be empty at some point, but would it make sense to do the check, when we interpret the result?
Looking at the tapo app source code, I see a few fields that might be useful:
temp_unit
string (it should probably be converted to an enum)min_control_temp
integermax_control_temp
integermin_temp
integer (it ends up getting used by frost protection)You're correct. ApiClient::control_child
should either be updated to handle empty responses or a copy of the function that can should be created.
Can you please run the example with RUST_LOG=debug
in front and paste the inner response you get?
For example, get_trigger_logs
returns something like
Device inner response decrypted: {"result":{"responseData":{"result":{"responses":[{"method":"get_trigger_logs","result":{"start_id":0,"logs":[],"sum":0},"error_code":0}]}}},"error_code":0}
Device inner response: TapoResponse { error_code: 0, result: Some(ControlChildResult { response_data: TapoMultipleResponse { result: TapoMultipleResult { responses: [TapoResponse { error_code: 0, result: Some(TriggerLogsResult { start_id: 0, sum: 0, logs: [] }) }] } } }) }
This what I get when executing device.set_temperature(19).await?;
:
2023-10-20T06:07:48.307Z INFO tapo_ke100 > EXECUTING COMMAND WITH EMPTY RESULT
2023-10-20T06:07:48.307Z DEBUG tapo::api::api_client > Control child...
2023-10-20T06:07:48.307Z DEBUG tapo::api::protocol::passthrough_protocol > Request to passthrough: {"method":"control_child","params":{"device_id":"REDACTED","requestData":{"method":"multipleRequest","params":{"requests":[{"method":"set_device_info","params":{"device_on":true,"target_temp":19}}]}}}}
2023-10-20T06:07:50.506Z DEBUG tapo::api::protocol::passthrough_protocol > Device responded with: TapoResponse { error_code: 0, result: Some(TapoResult { response: "kSb5Ej1g6e5y/FqhXbR8zs2pbesSXEoFnpur25O4SvYOAZIpvJ6bXrBACLOoaxeYHShR60TbXG1bad+r/jBY1vjKaAvEN3TYXqkxTTHp051EIBT88l1z1rV538LWPgGcLy+hD1rUdDX0wMO/p7HvDwMPIv6wxhnG/NwGxaTxt8g=" }) }
2023-10-20T06:07:50.506Z DEBUG tapo::api::protocol::passthrough_protocol > Device inner response decrypted: {"result":{"responseData":{"result":{"responses":[{"method":"set_device_info","error_code":0}]}}},"error_code":0}
2023-10-20T06:07:50.507Z DEBUG tapo::api::protocol::passthrough_protocol > Device inner response: TapoResponse { error_code: 0, result: Some(ControlChildResult { response_data: TapoMultipleResponse { result: TapoMultipleResult { responses: [TapoResponse { error_code: 0, result: None }] } } }) }
2023-10-20T06:07:50.507Z DEBUG tapo::api::protocol::passthrough_protocol > validate_response passed
2023-10-20T06:07:50.507Z DEBUG tapo::api::protocol::passthrough_protocol > assign result passed
What do you think about splitting ApiClient::control_child
into ApiClient::read_child
and ApiClient::write_child
to handle the None
response for the write operation? I'm struggling a bit to make ApiClient::control_child
handle both cases.
This should get the job done.
api_client
pub(crate) async fn control_child<R>(
&self,
device_id: String,
child_request: TapoRequest,
) -> Result<Option<R>, Error>
where
R: fmt::Debug + DeserializeOwned + TapoResponseExt,
{
debug!("Control child...");
let params = MultipleRequestParams::new(vec![child_request]);
let request = TapoRequest::MultipleRequest(Box::new(TapoParams::new(params)));
let params = ControlChildParams::new(device_id, request);
let request = TapoRequest::ControlChild(Box::new(TapoParams::new(params)));
let responses = self
.protocol
.execute_request::<ControlChildResult<TapoMultipleResponse<R>>>(request, true)
.await?
.ok_or_else(|| Error::Tapo(TapoResponseError::EmptyResult))?
.response_data
.result
.responses;
let response = responses
.into_iter()
.next()
.ok_or_else(|| Error::Tapo(TapoResponseError::EmptyResult))?;
validate_response(&response)?;
Ok(response.result)
}
hub_handler
pub(crate) async fn control_child<R>(
&self,
device_id: String,
request_data: TapoRequest,
) -> Result<Option<R>, Error>
where
R: fmt::Debug + DeserializeOwned + TapoResponseExt,
{
self.client.control_child(device_id, request_data).await
}
s200b_handler
pub async fn get_device_info(&self) -> Result<S200BResult, Error> {
let request = TapoRequest::GetDeviceInfo(TapoParams::new(EmptyParams));
self.hub_handler
.control_child(self.device_id.clone(), request)
.await?
.ok_or_else(|| Error::Tapo(TapoResponseError::EmptyResult))
}
Thanks for the code snippets. I managed to resolve the issue with the None
result in 8716347a4efdd0f6021316116e89eacc6218fde1. Need to work a bit more on this to support temperatures in float and add the parameters you suggested above.
Meanwhile I added support for min/max_control temp, child_protection and temp_offset.
I added a min_temp
parameter, but although the API doesn't respond with an error when setting a value the value on the device doesn't change (checked in the app). What is strange, is that neither device_info nor component_list seem to expose the min_temp parameter.
I'm thinking of adding a function to query the current temp directly for convenience as this will most likely be a common use case, apart from setting the temperature.
Let me know if you have any other thoughts on what could be added.
You're correct.
There's a separate endpoint called set_frost_protection
that requires min_temp
and temp_unit
as parameters. It's up to you if you want to include it in this pull request. Alternatively, we could save it for a future update to keep this one more manageable.
By the way, there's also a get_frost_protection
endpoint that I suspect will return the min_temp
.
However, It would be useful to include temp_unit
.
Regarding your question, there's a separate endpoint called get_temperature_records
that should fulfill the task. You can implement it in a similar way to get_trigger_logs
.
I've tried the changes you proposed but seem to be getting a compilation error in tapo/src/api/child_devices/ke100_handler.rs
now:
error[E0698]: type inside `async fn` body must be known in this context
--> tapo/src/api/child_devices/ke100_handler.rs:63:14
|
63 | .control_child(self.device_id.clone(), request)
| ^^^^^^^^^^^^^ cannot infer type for type parameter `R` declared on the method `control_child`
|
note: the type is part of the `async fn` body because of this `await`
I suggest we try to get this one fixed and then close this PR. I'll keep working on the other stuff and will create another PR once ready.
Thanks for the help with getting the Rust code right. As you may have recognized I'm learning by doing here.
Never mind, I fixed it. If I understood the Rust documentation right, I needed to bind control_child: .control_child::<KE100Result>
.
pub async fn set_temperature(&self, target_temperature: u8) -> Result<(), Error> {
let control_range = self.get_control_range().await?;
if target_temperature < control_range[0] || target_temperature > control_range[1] {
return Err(Error::Validation {
field: "target_temperature".to_string(),
message: format!("Target temperature must be between {} (min_control_temp) and {} (max_control_temp)", control_range[0], control_range[1]),
});
}
let json = serde_json::to_value(TrvSetDeviceInfoParams::new().target_temp(target_temperature)?)?;
let request = TapoRequest::SetDeviceInfo(Box::new(TapoParams::new(json)));
self.hub_handler
.control_child::<KE100Result>(self.device_id.clone(), request)
.await?;
Ok(())
}
I assume I can use the same pattern for all the functions which don't return a result?
I assume I can use the same pattern for all the functions which don't return a result?
Exactly, yeah. However, it would be better to use .control_child::<serde_json::Value>
instead of .control_child::<KE100Result>
.
Please give me a shout when you're done with everything you want to do, and I'll do a review. If you don't have the time to address all the little things, you can permit me to contribute to the PR.
Just pushed the last updates. I think the PR is ready to go for review.
I found one more thing I may need your help with. It's about decoding the base64 encoded nickname. The following fragment in example/tapo_ke100.rs
returns the base64 encoded nickname:
let hub = ApiClient::new(tapo_username, tapo_password)?
.h100(ip_address)
.await?;
// Get a handler for the child device
let device = hub.ke100(device_id);
// Get the device info of the child device
let device_info = device.get_device_info().await?;
info!("Device info: {device_info:?}");
But when I use the detour via get_child_device_list()
as in the following fragment, I get the decoded nickname in device.nickname
// Adapted code from examples/tapo_h100.rs
let hub = ApiClient::new(tapo_username, tapo_password)?
.h100(ip_address)
.await?;
let child_device_list = hub.get_child_device_list().await?;
for child in child_device_list {
match child {
ChildDeviceResult::KE100(device) => {
info!(
"Found KE100 child device with nickname: {}, id: {}, current temperature: {} {:?} and target temperature: {} {:?}.",
device.nickname,
device.device_id,
device.current_temperature,
device.temperature_unit,
device.target_temperature,
device.temperature_unit,
);
}
Do you see a similar behavior, when you test other child devices such as s200b or t300? Is this intended behavior or should we do something about this?
Ah, that is a defect. Nice find! I've fixed it in https://github.com/mihai-dinculescu/tapo/commit/fa73e407e578dc748a4a4bca9f6528f4a18649e4.
Merged your fixes for the nickname decoding into this PR. The decoding seems to work also with the feature I added to supported empty results for the control_child
function.
I've committed some minor tweaks for language and lints. I think this is in a good shape to ship 🚀
Released in v0.7.6. Thank you for this amazing effort!
Reading data works. Setting temperature and frost protection returns empty result from the device, which creates an EmptyResult runtime error.