Skip to content

Improve loop handling#270

Merged
ikalchev merged 4 commits intoikalchev:devfrom
balloob:better-loop-handling
Jul 4, 2020
Merged

Improve loop handling#270
ikalchev merged 4 commits intoikalchev:devfrom
balloob:better-loop-handling

Conversation

@balloob
Copy link
Copy Markdown
Contributor

@balloob balloob commented Jul 3, 2020

This improves loop handling for HAP-Python:

  • Do not change the default executor of passed in loops
  • Do not shut down loops/executor pools that were passed in

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 3, 2020

Codecov Report

Merging #270 into dev will decrease coverage by 0.37%.
The diff coverage is 72.22%.

@@            Coverage Diff             @@
##              dev     #270      +/-   ##
==========================================
- Coverage   64.50%   64.13%   -0.38%     
==========================================
  Files          16       16              
  Lines        1761     1751      -10     
  Branches      189      191       +2     
==========================================
- Hits         1136     1123      -13     
- Misses        572      573       +1     
- Partials       53       55       +2     
Impacted Files Coverage Δ
pyhap/accessory_driver.py 69.28% <72.22%> (-0.79%) ⬇️
pyhap/accessory.py 56.16% <0.00%> (-0.60%) ⬇️
pyhap/camera.py 77.20% <0.00%> (-0.25%) ⬇️
pyhap/loader.py 90.00% <0.00%> (-0.25%) ⬇️
pyhap/hap_server.py 38.65% <0.00%> (-0.13%) ⬇️
pyhap/service.py 98.11% <0.00%> (-0.04%) ⬇️
pyhap/state.py 100.00% <0.00%> (ø)
pyhap/encoder.py 100.00% <0.00%> (ø)
pyhap/characteristic.py 100.00% <0.00%> (ø)

@bdraco
Copy link
Copy Markdown
Contributor

bdraco commented Jul 3, 2020

            if threading.current_thread() is threading.main_thread():
                logger.debug('Setting child watcher')
                watcher = asyncio.SafeChildWatcher()
                watcher.attach_loop(self.loop)
                asyncio.set_child_watcher(watcher)

I think we need a way to call _do_start without the above in start

@balloob
Copy link
Copy Markdown
Contributor Author

balloob commented Jul 3, 2020

Done, renamed _do_start to start_service

@bdraco
Copy link
Copy Markdown
Contributor

bdraco commented Jul 3, 2020

Everything appears to work as expected in the linked PR (testing using the existing protected methods)

@bdraco
Copy link
Copy Markdown
Contributor

bdraco commented Jul 3, 2020

self.topic_lock = threading.Lock() # for exclusive access to the topics
self.loader = loader or Loader()
self.aio_stop_event = asyncio.Event(loop=self.loop)
self.aio_stop_event = asyncio.Event()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is breaking Python 3.5 support. It's not necessary, just a drive by cleanup from when I saw warnings printed during tests. I will revert it.

Copy link
Copy Markdown
Contributor

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified working with home-assistant/core#37441

@bdraco
Copy link
Copy Markdown
Contributor

bdraco commented Jul 3, 2020

Verified this doesn't regress busy home example

@bdraco
Copy link
Copy Markdown
Contributor

bdraco commented Jul 3, 2020

@ikalchev If this one looks good, could we get a new release?

Thank you.

@ikalchev ikalchev merged commit b194c63 into ikalchev:dev Jul 4, 2020
@ikalchev
Copy link
Copy Markdown
Owner

ikalchev commented Jul 4, 2020

Change is good, will release in the evening.

Best,
Ivan

@bdraco
Copy link
Copy Markdown
Contributor

bdraco commented Jul 4, 2020

Thanks Ivan 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants