Skip to content

Fixing bug #180 #188

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixing bug #180 #188

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d1e7c6
Fixing bug #180
elechapt Jun 26, 2023
82a4a5f
Switch comments
elechapt Jun 26, 2023
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
13 changes: 10 additions & 3 deletions picoscope/picobase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,9 +881,16 @@ def close(self):
might take some time.

"""
if self.handle is not None:
self._lowLevelCloseUnit()
self.handle = None
try:
if self.handle is not None:
self._lowLevelCloseUnit()
self.handle = None
except AttributeError:
# _lowLevelCloseUnit raise: libps5000a.so not found if not connected
pass
except OSError:
# self.handle doesn't exist when not connected
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't this raise an attribute error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sorry, I switched the comments ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

what does "handle does not exist" mean? isn't the handle None in that case? in which case the if statement takes care of it.

Copy link
Contributor Author

@elechapt elechapt Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's what I thought, but I ended up writing some code where I get an AttributeError stating that PS5000a does not have an attribute named handle, which means at some point, the garbage collector has destroyed the attribute, but can still call the __del__ method. It already happened to me in an unrelated piece of code, I was handling threads and somehow, at deletion, methods could be called but attributes weren't there anymore.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a common issue with adding definitions outside the init method.

Let's then define handle as None in the init method and then we can get rid of this one except issue.

Copy link
Contributor Author

@elechapt elechapt Jun 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No, it's already defined in the __init__. After some looking up, I conclude that it is linked to the behaviour of __del__, where the simple attributes are already destroyed when arriving in the __del__ method. To quote the __del__ doc :

It is not guaranteed that del() methods are called for objects that still exist when the interpreter exits.

After some extra reading, it turns out that __del__ is at best a makeshift, at worse completely useless, it's better to call the close() method anyway. __del__ is intrinsically unreliable.

pass

def stop(self):
"""Stop scope acquisition."""
Expand Down