Use query string for set_zone on airbase #11
Conversation
|
Please make sure |
I think it should all be good - could we get the version bumped with these changes ? |
|
@fredrike Is there anything that prevents this PR from being merged? |
I'm waiting for @kingy444 to do some more testing on why we can't use params when configuring zones. I do not have an airbase unit so I can't test and verify this myself. |
This PR was raised to fix the issue that params don't work. What else are you wanting me to test ? They don't work so this PR makes Airbase use query string (which does work) |
I really apprechiate your work so don't get me wrong the problem is that we need to understand what's going on here so we don't change this to param later on. I did write some comments here: https://github.com/fredrike/pydaikin/pull/11/files/70f1d1c6b4c44aef037594eb4a98a8d28ccb5677 |
|
All good from my end - I'm just not sure what else you need. All i can put it down to is the airbase unit must process these differently - any request with a POST body fails and it requires them in query string GET |
|
There are no POST going on, only get: https://github.com/fredrike/pydaikin/blob/master/pydaikin/daikin_base.py#L140 Please show me what's different with the request, enable verbose logging in HTTPX with: https://www.python-httpx.org/logging/ |
|
Well that explains why POSTMAN failed before... GET with params does seems to be working. Query String Params
All this led me to check the response url in the daikin_base which returned This was returning current_group = self.represent(key)[1]
current_group[zone_id] = value
self.values[key] = quote(";".join(current_group)).lower()
path = "aircon/set_zone_setting"
params = {
"zone_name": unquote(current_state["zone_name"]),
"zone_onoff": unquote(self.values["zone_onoff"]),
}
if self.support_zone_temperature:
params.update({"lztemp_c": unquote(self.values["lztemp_c"])})
params.update({"lztemp_h": unquote(self.values["lztemp_h"])})
await self._get_resource(path, params) |
|
These were all out of the box names - i forced them back on the device with the below params["zone_name"] = "%20%20%20%20%20%20%20%5a%6f%6e%65%31%3b%20%20%20%20%20%20%20%5a%6f%6e%65%32%3b%20%20%20%20%20%20%20%5a%6f%6e%65%33%3b%20%20%20%20%20%20%20%5a%6f%6e%65%34%3b%20%20%20%20%20%20%20%5a%6f%6e%65%35%3b%20%20%20%20%20%20%20%5a%6f%6e%65%36%3b%20%20%20%20%20%20%20%5a%6f%6e%65%37%3b%20%20%20%20%20%20%20%5a%6f%6e%65%38"they are padded with spaces when converted |
|
so it looks like everything within at the start of but then within the response block the url called is url encoded again rather than treating as url encoded already ( as above, when trying to decode the thoughts? |
Great findings and analysis! I would guess that we need to use So httpx can do an 
|
|
@kingy444 I think you've nailed it. The Daikin implementation does it wrong and expect the string to be urlencoded but that is not according to standard. Some references: https://stackoverflow.com/questions/1634271/url-encoding-the-space-character-or-20, encode/httpx#3140 I think we can drop this and keep it as you suggested it just add a comment that it shouldn't be sent as parameters as Daikin doesn't parse them correctly. Or, this actually fixes our issue: https://github.com/encode/httpx/pull/3187/files#diff-e22e5624ca4e994155aa44e1b25f7812b286a8bbbeabdcd6490073ad2f3927dcR293 This is the way: 
We add requirement of httpx version greater than 0.23.0 
And now I realized that we are using aiohttp, I'll get back on this. |
|
So, aiohttp is using yarl and this is how yarl encodes our string: 
So I guess we are back to where we started, we need to keep the query string separate.. |
| _LOGGER.debug("Sending request to %s with params: %s", path, params) | ||
| await self._get_resource(path, params) | ||
| # # Convert params dictionary to query string format | ||
| params_str = "&".join(f"{k}={v}" for k, v in params.items()) |
There was a problem hiding this comment.
I do think we should try to get the params way to work. Can you add some debugging on what's happening here in contrast to the params way?
Also it seems like httpx are using https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode. Isn't that a better way?
There was a problem hiding this comment.
@kingy444 could you try to run a few scenarios that uses the inbuilt params in httpx. I would like to understand what's going on here. Perhas you could enable verbose (NOTSET) debugging on httpx so we can see how the request differs.
There was a problem hiding this comment.
@kingy444 could you have a look at this ⬆️ .
|
Thanks a lot @kingy444! New version (2.13.1) pushed to pypi. |
Params do not work for the set_zone function. Revert to Query string
fixes #10
Please bump version and publish after merge, an incremental to 2.13.1 should be sufficient