Added a check when performing a post. #102
Conversation
If a device control was performed, release the control back. Also refactored the logic a bit so the both the api_root and path are passed to the post method
80 chars is so 90ies -_-
|
80 chars FFS. People have widescreen monitors these days... Not saying lines can be 200 long but 80 is like WTF |
|
Missed a line! 🙃 For real though, 100 or 120 if any at all is much saner these days |
There was a problem hiding this comment.
Thanks for getting this started! I have a couple of comments within, and I would love to not add the extra parameter to lgedm_post for now. Instead, let's just send a second lgedm_post from the locations that need it.
One other thing I'm worried about is that this will be wasteful for some situations… that is, every request will now turn into two requests. For some situations where you want to make multiple requests, it seems bad that we'll now be doing 2n service requests instead of n+1 (the original n requests, plus just one delControlPermission at the very end). Probably not a big deal, but maybe worth worrying about… we could also require clients to explicitly relinquish control, which would avoid the problem.
| @@ -12,7 +12,7 @@ | |||
| from requests.adapters import HTTPAdapter | |||
| from requests.packages.urllib3.util.retry import Retry | |||
|
|
|||
| GATEWAY_URL = 'https://kic.lgthinq.com:46030/api/common/gatewayUriList' | |||
| GATEWAY_URL = 'https://kic.lgthinq.com:46030' | |||
There was a problem hiding this comment.
Let's leave this change where we split up the base URL and path out for now—it looks interesting, but I would love to review it carefully in a separate PR.
| res = session.post(urljoin(api_root + '/', path), | ||
| json={DATA_ROOT: data}, headers=headers) | ||
|
|
||
| if "rtiControl" in path: |
There was a problem hiding this comment.
Let's move this from lgedm_post, which is a low-level communication primitive, to the places that actually use rtiControl. That way we won't have to check what kind of request we're doing; we'll do it in exactly the right place "by construction."
I think that might juts mean set_device_controls, or possibly that and get_device_config. Can we try it with just the former, and see if it works, and if not, do the latter?
|
From my experience, a delControlPermission is needed for every control request sent, otherwise the oofficial mobile app locks out for a while |
|
Sure, makes sense—but let's put it where we initiate those |
|
Understandable! |
If a device control was performed, release the control back. Also refactored the logic a bit so the both the api_root and path are passed to the post method