Skip to content

Commit ff8b4a9

Browse files
ehendrix23ehendrix23
committed
Fixed race condition when opening/closing in rapid
fashion.
1 parent 672d0a2 commit ff8b4a9

File tree

5 files changed

+132
-126
lines changed

5 files changed

+132
-126
lines changed

‎example.py‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
EMAIL = "<EMAIL>"
1515
PASSWORD = "<PASSWORD>"
16-
ISSUE_COMMANDS = False
16+
ISSUE_COMMANDS = True
17+
# LOGLEVEL = logging.DEBUG
18+
LOGLEVEL = logging.WARNING
1719

1820

1921
def print_info(number: int, device):
@@ -41,7 +43,7 @@ def print_info(number: int, device):
4143
print(" ---------")
4244

4345

44-
async def print_garagedoors(account: MyQAccount):
46+
async def print_garagedoors(account: MyQAccount): # noqa: C901
4547
"""Print garage door information and open/close if requested
4648
4749
Args:
@@ -55,50 +57,63 @@ async def print_garagedoors(account: MyQAccount):
5557

5658
if ISSUE_COMMANDS:
5759
try:
60+
open_task = None
61+
opened = closed = False
5862
if device.open_allowed:
5963
if device.state == STATE_OPEN:
6064
print(f"Garage door {device.name} is already open")
6165
else:
6266
print(f"Opening garage door {device.name}")
6367
try:
64-
if await device.open(wait_for_state=True):
65-
print(f"Garage door {device.name} has been opened.")
66-
else:
67-
print(f"Failed to open garage door {device.name}.")
68+
open_task = await device.open(wait_for_state=False)
6869
except MyQError as err:
6970
_LOGGER.error(
7071
"Error when trying to open %s: %s",
7172
device.name,
7273
str(err),
7374
)
75+
print(f"Garage door {device.name} is {device.state}")
76+
7477
else:
7578
print(f"Opening of garage door {device.name} is not allowed.")
7679

80+
# We're not waiting for opening to be completed before we do call to close.
81+
# The API will wait automatically for the open to complete before then
82+
# processing the command to close.
83+
7784
if device.close_allowed:
7885
if device.state == STATE_CLOSED:
7986
print(f"Garage door {device.name} is already closed")
8087
else:
81-
print(f"Closing garage door {device.name}")
82-
wait_task = None
88+
if open_task is None:
89+
print(f"Closing garage door {device.name}")
90+
else:
91+
print(
92+
f"Already requesting closing garage door {device.name}"
93+
)
94+
8395
try:
84-
wait_task = await device.close(wait_for_state=False)
96+
closed = await device.close(wait_for_state=True)
8597
except MyQError as err:
8698
_LOGGER.error(
8799
"Error when trying to close %s: %s",
88100
device.name,
89101
str(err),
90102
)
91103

92-
print(f"Device {device.name} is {device.state}")
93-
94-
if (
95-
wait_task
96-
and isinstance(wait_task, asyncio.Task)
97-
and await wait_task
98-
):
99-
print(f"Garage door {device.name} has been closed.")
100-
else:
101-
print(f"Failed to close garage door {device.name}.")
104+
if open_task is not None and not isinstance(open_task, bool):
105+
opened = await open_task
106+
107+
if opened and closed:
108+
print(
109+
f"Garage door {device.name} was opened and then closed again."
110+
)
111+
elif opened:
112+
print(f"Garage door {device.name} was opened but not closed.")
113+
elif closed:
114+
print(f"Garage door {device.name} was closed but not opened.")
115+
else:
116+
print(f"Garage door {device.name} was not opened nor closed.")
102117

103118
except RequestError as err:
104119
_LOGGER.error(err)
@@ -161,7 +176,7 @@ async def print_other(account: MyQAccount):
161176

162177
async def main() -> None:
163178
"""Create the aiohttp session and run the example."""
164-
logging.basicConfig(level=logging.DEBUG)
179+
logging.basicConfig(level=LOGLEVEL)
165180
async with ClientSession() as websession:
166181
try:
167182
# Create an API object:

‎pymyq/__version__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
"""Define a version constant."""
2-
__version__ = "3.1.0b4"
2+
__version__ = "3.1.0b5"

‎pymyq/device.py‎

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ def __init__(
3030
self.last_state_update = state_update
3131
self.state_update = None
3232
self._device_state = None # Type: Optional[str]
33+
self._send_command_lock = asyncio.Lock() # type: asyncio.Lock
34+
self._wait_for_state_task = None
3335

3436
@property
3537
def account(self) -> "MyQAccount":
@@ -152,20 +154,76 @@ async def update_device(self, device_json: dict, state_update_timestmp: datetime
152154

153155
self.state_update = state_update_timestmp
154156

155-
async def _send_state_command(self, url: str, command: str) -> None:
156-
"""Instruct the API to change the state of the device."""
157+
async def _send_state_command(
158+
self,
159+
to_state: str,
160+
intermediate_state: str,
161+
url: str,
162+
command: str,
163+
wait_for_state: bool = False,
164+
) -> Union[asyncio.Task, bool]:
165+
"""Send command to device to change state."""
166+
157167
# If the user tries to open or close, say, a gateway, throw an exception:
158168
if not self.state:
159169
raise RequestError(
160170
f"Cannot change state of device type: {self.device_type}"
161171
)
162172

163-
_LOGGER.debug("Sending command %s for %s", command, self.name)
164-
await self.account.api.request(
165-
method="put",
166-
returns="response",
167-
url=url,
168-
)
173+
async with self._send_command_lock:
174+
# If currently there is a wait_for_state task running,
175+
# then wait until it completes first.
176+
if self._wait_for_state_task is not None:
177+
# Return wait task if we're currently waiting for same task to be completed
178+
if self.state == intermediate_state and not wait_for_state:
179+
_LOGGER.debug(
180+
"Command %s for %s was already send, returning wait task for it instead",
181+
command,
182+
self.name,
183+
)
184+
return self._wait_for_state_task
185+
186+
_LOGGER.debug(
187+
"Another command for %s is still in progress, waiting for it to complete first before issuing command %s",
188+
self.name,
189+
command,
190+
)
191+
await self._wait_for_state_task
192+
193+
# We return true if state is already closed.
194+
if self.state == to_state:
195+
_LOGGER.debug(
196+
"Device %s is in state %s, nothing to do.", self.name, to_state
197+
)
198+
return True
199+
200+
_LOGGER.debug("Sending command %s for %s", command, self.name)
201+
await self.account.api.request(
202+
method="put",
203+
returns="response",
204+
url=url,
205+
)
206+
207+
self.state = intermediate_state
208+
209+
self._wait_for_state_task = asyncio.create_task(
210+
self.wait_for_state(
211+
current_state=[self.state],
212+
new_state=[to_state],
213+
last_state_update=self.device_json["state"].get("last_update"),
214+
timeout=60,
215+
),
216+
name="MyQ_WaitFor" + to_state,
217+
)
218+
219+
# Make sure our wait task starts
220+
await asyncio.sleep(0)
221+
222+
if not wait_for_state:
223+
return self._wait_for_state_task
224+
225+
_LOGGER.debug("Waiting till device is %s", to_state)
226+
return await self._wait_for_state_task
169227

170228
async def update(self) -> None:
171229
"""Get the latest info for this device."""
@@ -209,7 +267,7 @@ async def wait_for_state(
209267
# Wait until the state is to what we want it to be
210268
_LOGGER.debug("Waiting until device state for %s is %s", self.name, new_state)
211269
wait_timeout = timeout
212-
while self.state in current_state and wait_timeout > 0:
270+
while self.device_state not in new_state and wait_timeout > 0:
213271
wait_timeout = wait_timeout - 5
214272
try:
215273
await self._account.update()
@@ -221,7 +279,8 @@ async def wait_for_state(
221279
# Reset self.state ensuring it reflects actual device state.
222280
# Only do this if state is still what it would have been,
223281
# this to ensure if something else had updated it to something else we don't override.
224-
if self._device_state == current_state:
282+
if self._device_state in current_state or self._device_state in new_state:
225283
self._device_state = None
226284

285+
self._wait_for_state_task = None
227286
return self.state in new_state

‎pymyq/garagedoor.py‎

Lines changed: 17 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from typing import TYPE_CHECKING, Optional, Union
66

77
from .device import MyQDevice
8-
from .errors import RequestError
98

109
if TYPE_CHECKING:
1110
from .account import MyQAccount
@@ -23,7 +22,6 @@
2322
STATE_OPEN = "open"
2423
STATE_OPENING = "opening"
2524
STATE_STOPPED = "stopped"
26-
STATE_TRANSITION = "transition"
2725
STATE_UNKNOWN = "unknown"
2826

2927

@@ -64,72 +62,30 @@ def device_state(self) -> Optional[str]:
6462

6563
async def close(self, wait_for_state: bool = False) -> Union[asyncio.Task, bool]:
6664
"""Close the device."""
67-
if self.state != self.device_state:
68-
raise RequestError(
69-
f"Device is currently {self.state}, wait until complete."
70-
)
7165

72-
if self.state not in (STATE_CLOSED, STATE_CLOSING):
73-
# If our state is different from device state,
74-
# then it means an action is already being performed.
75-
if self.state != self.device_state:
76-
raise RequestError(
77-
f"Device is currently {self.state}, wait until complete."
78-
)
79-
80-
# Device is currently not closed or closing, send command to close
81-
await self._send_state_command(
82-
url=COMMAND_URI.format(
83-
account_id=self.account.id,
84-
device_serial=self.device_id,
85-
command=COMMAND_CLOSE,
86-
),
66+
return await self._send_state_command(
67+
to_state=STATE_CLOSED,
68+
intermediate_state=STATE_CLOSING,
69+
url=COMMAND_URI.format(
70+
account_id=self.account.id,
71+
device_serial=self.device_id,
8772
command=COMMAND_CLOSE,
88-
)
89-
self.state = STATE_CLOSING
90-
91-
wait_for_state_task = asyncio.create_task(
92-
self.wait_for_state(
93-
current_state=[STATE_CLOSING],
94-
new_state=[STATE_CLOSED],
95-
last_state_update=self.device_json["state"].get("last_update"),
96-
timeout=60,
9773
),
98-
name="MyQ_WaitForClose",
74+
command=COMMAND_CLOSE,
75+
wait_for_state=wait_for_state,
9976
)
100-
if not wait_for_state:
101-
return wait_for_state_task
102-
103-
_LOGGER.debug("Waiting till garage is closed")
104-
return await wait_for_state_task
10577

10678
async def open(self, wait_for_state: bool = False) -> Union[asyncio.Task, bool]:
10779
"""Open the device."""
108-
if self.state not in (STATE_OPEN, STATE_OPENING):
109-
# Set the current state to "opening" right away (in case the user doesn't
110-
# run update() before checking):
111-
await self._send_state_command(
112-
url=COMMAND_URI.format(
113-
account_id=self.account.id,
114-
device_serial=self.device_id,
115-
command=COMMAND_OPEN,
116-
),
117-
command=COMMAND_OPEN,
118-
)
119-
self.state = STATE_OPENING
12080

121-
wait_for_state_task = asyncio.create_task(
122-
self.wait_for_state(
123-
current_state=[STATE_OPENING],
124-
new_state=[STATE_OPEN],
125-
last_state_update=self.device_json["state"].get("last_update"),
126-
timeout=60,
81+
return await self._send_state_command(
82+
to_state=STATE_OPEN,
83+
intermediate_state=STATE_OPENING,
84+
url=COMMAND_URI.format(
85+
account_id=self.account.id,
86+
device_serial=self.device_id,
87+
command=COMMAND_OPEN,
12788
),
128-
name="MyQ_WaitForOpen",
89+
command=COMMAND_OPEN,
90+
wait_for_state=wait_for_state,
12991
)
130-
131-
if not wait_for_state:
132-
return wait_for_state_task
133-
134-
_LOGGER.debug("Waiting till garage is open")
135-
return await wait_for_state_task

0 commit comments

Comments
 (0)