h4 / lywsd02

MIT License
169 stars 34 forks source link

Avoid unwanted disconnect when asking time from a connect context #19

Closed andras-tim closed 4 years ago

h4 commented 4 years ago

I'm not sure that this solution is clear enough. Could you describe what problem are you trying to solve, step by step? Looks like more elegant solution should be exist.

andras-tim commented 4 years ago

Of course!

Here is the code what can reproduce the unwanted behavior:

import logging
from datetime import datetime
from lywsd02 import Lywsd02Client

logging.basicConfig(level=logging.DEBUG, format='%(asctime)s %(message)s')

with Lywsd02Client('XXX').connect() as client:
    logging.info('sync time')
    client.time = datetime.now()
    logging.info('check sync result')
    print(client.time)
2020-04-27 14:40:03,471 Connecting to XXX            <= expected connect (by context manager enter)
2020-04-27 14:40:06,738 sync time
2020-04-27 14:40:10,807 Disconnecting from XXX       <= not expected (by an internat context manager exit in the `time` setter)
2020-04-27 14:40:10,809 check sync result
2020-04-27 14:40:10,809 Connecting to XXX            <= not expected (by an internat context manager enter in the `time` getter)
2020-04-27 14:40:15,487 Disconnecting from XXX       <= not expected (by an internat context manager exit in the `time` getter)
(datetime.datetime(2020, 4, 27, 14, 40, 11), 2)
                                                     <= missing disconnect (by context manager exit)

Notes:

andras-tim commented 4 years ago

@h4, do you have any news? At first, what was the main reason behind you think this solution is dirty? I would like to think w/ you and solve the problem as nice and as fast as possible.

h4 commented 4 years ago

I'm hoping I could work on this task on this weekend, right now i'm quiet busy with my job.

I think, this condition is confusing https://github.com/h4/lywsd02/pull/19/files#diff-a734af08fd6de172d3e446f81d39aa87R55

Maybe it will be better to use something like transaction or call connect/disconnect directly. I'm not sure which options is the best one.

h4 commented 4 years ago

@andras-tim I've just released 0.0.9. In this versions we connect to peripheral only in outer context manager.

Thank you so much for inspiration!

andras-tim commented 4 years ago

@andras-tim I've just released 0.0.9. In this versions we connect to peripheral only in outer context manager.

Thank you so much for inspiration!

You're welcome! :slightly_smiling_face:

But maybe I don't understand something, I can't find the "magic" in [the diff].(https://github.com/h4/lywsd02/compare/357718431453564834d282fbd92cca85d6306a8b...master) Are you sure, did you released the fix?

h4 commented 4 years ago

Ouch! I send release to pypi but forgot to push source code. Fixed.

andras-tim commented 4 years ago

Nice fix! :slightly_smiling_face: Thank you!