Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cirque - update int/pointer casting and reset subscription counts in tests. #10771

Merged
merged 2 commits into from
Oct 21, 2021
Merged

Fix cirque - update int/pointer casting and reset subscription counts in tests. #10771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39c7433
Fix GetConnectedDeviceSync
cecille Oct 20, 2021
0e31792
Fix Subscription test.
cecille Oct 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/controller/python/chip/ChipDeviceCtrl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def DeviceAvailableCallback(device, err):
nonlocal returnDevice
nonlocal deviceAvailableCV
with deviceAvailableCV:
returnDevice = device
returnDevice = c_void_p(device)
deviceAvailableCV.notify_all()
if err != 0:
print("Failed in getting the connected device: {}".format(err))
Expand All @@ -330,11 +330,11 @@ def DeviceAvailableCallback(device, err):

# The callback might have been received synchronously (during self._ChipStack.Call()).
# Check if the device is already set before waiting for the callback.
if returnDevice == c_void_p(None):
if returnDevice.value == None:
with deviceAvailableCV:
deviceAvailableCV.wait()

if returnDevice == c_void_p(None):
if returnDevice.value == None:
raise self._ChipStack.ErrorToException(CHIP_ERROR_INTERNAL)
return returnDevice

Expand Down
3 changes: 3 additions & 0 deletions src/controller/python/test/test_scripts/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,14 @@ def run(self):
"OnOff", "OnOff", nodeid, endpoint, 1, 10)
changeThread = _conductAttributeChange(
self.devCtrl, nodeid, endpoint)
# Reset the number of subscriptions received as subscribing causes a callback.
handler.subscriptionReceived = 0
changeThread.start()
with handler.cv:
while handler.subscriptionReceived < 5:
# We should observe 10 attribute changes
Comment on lines 351 to 352
Copy link
Contributor

@tcarmelveilleux tcarmelveilleux Oct 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The comment seems to mismatch intent. Is there a bug here?

@yunhanw-google

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably just a bug.

handler.cv.wait()
changeThread.join()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you comment on the join reason? Could this wait forever on failed test? Should we have a timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That thread won't fail to join unless the send message call hangs, in which case we're already sunk in this test. The real challenge here is the handler.subscriptionReceived never hitting 5, which will happen if the two threads get out of sync with how many subscriptions they're looking for. But we already have that condition in here, and there's a timer on the tests anyway. I added this because before the thread was continuing along and sending message while other parts of the test were going and it was causing some interesting behaviour. That's rather a different problem (ie, we should add tests for what happens when you fire just a whole lot of requests to the node), but that's orthogonal to this issue.

return True
except Exception as ex:
self.logger.exception(f"Failed to finish API test: {ex}")
Expand Down