bug fixes #81
bug fixes #81
Conversation
…nts user execution to wait
aircon/__main__.py
Outdated
| await asyncio.sleep(_STATUS_UPDATE_INTERVAL) | ||
|
|
||
| async def query_status_worker(devices: [Device]): | ||
| await asyncio.gather(*(query_status_device(device) |
There was a problem hiding this comment.
You don't really have anything to gather here, use asyncio.wait instead.
aircon/notifier.py
Outdated
| @@ -116,9 +116,14 @@ async def _perform_request(self, session: aiohttp.ClientSession, | |||
| resp_data = await resp.text() | |||
| logging.error(f'[KeepAlive] Sending local_reg failed: {resp.status}, {resp_data}') | |||
| raise ConnectionError(f'Sending local_reg failed: {resp.status}, {resp_data}') | |||
| except aiohttp.client_exceptions.ClientConnectionError as e: | |||
There was a problem hiding this comment.
Just do:
except (aiohttp.client_exceptions.ClientConnectionError, aiohttp.client_exceptions.ClientConnectorError) as e:
aircon/aircon.py
Outdated
| for data_field in fields(self._properties): | ||
| while self.commands_queue.qsize() > 0: |
There was a problem hiding this comment.
Has this been a real issue?
The current implementation is pretty much a busy loop.
A cleaner option would be to just use a queue.PriorityQueue, that will contain something like:
@dataclass(order=True)
class Command:
priority: int
timestamp: int
command: Dict=field(compare=False)
updater: Callable=field(compare=False)(The timestamp should get time.time_ns(), it's there to maintain order for equal priority commands).
There was a problem hiding this comment.
That was the first issue I tried to solve.
When that busy loops run, you can't issue any command. And that happens every 10 minutes.
The solution I had, gave priority to other commands than the status.
PriorityQueue, I don't familiar with... It just has to give more priority to commands other than the status update.
You will still have to control the order of the commands.
There was a problem hiding this comment.
Right, so the idea is to use queue.PriorityQueue instead of queue.Queue, and set the priority to e.g. 10 for "normal" commands, and 100 for these status updates. Then any normal command will be prioritized over the status updates.
The queue orders the elements by a using __lt__, which is implemented by dataclass in field order (skipping fields with compare=False) - so just setting the correct values will do the right thing.
There was a problem hiding this comment.
Can you please merge this and change it? I think you're gonna handle this better and faster than me :) Meantime I try to investigate another issue I see with some of the AC which does not report the correct value.
There was a problem hiding this comment.
Sure, I can do it. but let's revert this change.
There was a problem hiding this comment.
Thanks. I've submitted 834fab2 for this, you can try it out - didn't get a chance to test it yet.
There was a problem hiding this comment.
Thanks :) it works as expected 👍
…y. Prevents user execution to wait" This reverts commit cb9c5f8.
Hi @deiger
Please see my commits for more details.
As you already know, I have 8 ACs here, and AirCon was dead slow when it was running through the status update.
Tested everything and it seems to work as it should.
You may merge it if you like.
I'm pretty satisfied with the outcome.
Now when the system loads and the status updates are running, I can send a command to the AC, before that, it was long in the queue.