Stability improvement with Home Assistant

[olalid]

Description:

Lock functions added such that only one command is sent and processed at the heos device before next command is sent. This improves stability when used with the HA integration hugely. From usually being disconnected randomly after 1-24 hours it has now yet to fail for me after several weeks of service.

See also #29 for additional stability improvements.

Related issue (if applicable): fixes #20

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• [N/A] Tests have been added/updated. No exclusions in .coveragerc permitted.
• [N/A] README.MD updated (if necessary)

[andrewsayre]

OK, so I've been looking more closely at these code changes. First, please break the lock and reconnect improvements into separate PRs as they aren't directly related to each other.

The lock only needs to be around these 3 lines (acquire the lock right before these lines, within the try block). Locks should cover the least amount of operations possible, and there's no need to have it earlier blocking the other synchronous code.

pyheos/pyheos/connection.py

Lines 271 to 273 in f500cf5

	self._writer.write((uri + SEPARATOR).encode())
	await self._writer.drain()
	response = await asyncio.wait_for(event.wait(), self.timeout)

The effect of the lock will result in a command blocking until it receives a response or times out. If this is really a limitation of HEOS devices (it can only process one command at a time), the sequence tracking and pending command queue logic becomes unnecessary and could to be simplified.

Have you tried placing the lock around just these two lines?

pyheos/pyheos/connection.py

Lines 271 to 272 in f500cf5

	self._writer.write((uri + SEPARATOR).encode())
	await self._writer.drain()

Perhaps the write is being called again while in a break within drain and causes errata over the connection?

I'll see if I can get in touch with HEOS engineering to see their perspective on synchronous vs. async commands.

Andy

[olalid]

I have tried placing the lock as you suggest, but it does not work since this below code line will then execute before the connection state is really up to date, i.e if the previous command fails (times out):

pyheos/pyheos/connection.py

Line 260 in f500cf5

if self._state != const.STATE_CONNECTED:

I can absolutely break it in to separate PRs, I will get back on that when I have time.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (dev@345d25e). Learn more about missing BASE report.

Additional details and impacted files

@@          Coverage Diff           @@
##             dev      #21   +/-   ##
======================================
  Coverage       ?   99.60%           
======================================
  Files          ?       19           
  Lines          ?     2281           
  Branches       ?        0           
======================================
  Hits           ?     2272           
  Misses         ?        9           
  Partials       ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[olalid]

@andrewsayre Now split in to 2 separate PRs.

[andrewsayre]

#34 addresses this feedback and implementation suggestions. Closing the PR!