Fixing bug #180

[elechapt]

Catch Exceptions in the _PicoscopeBase() class (picobase.py) that are raised when not connected. They do not convey useful information, as they are typical error happening with the garbage collector and disappearing attributes.

[hmaarrfk]

doesn't this raise an attribute error?

[elechapt]

Oh, sorry, I switched the comments ...

[hmaarrfk]

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.

[elechapt]

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.

[hmaarrfk]

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.

[elechapt]

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.

[hmaarrfk]

I'm not sure i agree with the fix.

I think the issue is that:

https://github.com/colinoflynn/pico-python/blob/master/picoscope/ps5000a.py#L172

c_handle.value

could return 0x0 and not Null. therefore we should be "checking" for that in the case of errors and handling that correctly.

[elechapt]

That sure can be a source of error, which raises maybe an OSError. I 'll try to fix that too.

But the AttributeError is very specific, and well documented. We actually should always call explicitly the close() method, and not rely on the __del__ method :

there is no guarantee that __del__() will be called: it's just for memory manager use, not for resource handling.

And after some testing, the real problem with the line you mention is that the next function is doing it's job, the close unit function raises after trying to connect to a given picoscope and failing. Actually, this happens :

• Try to open a picoscope, _lowLevelOpenUnit failing because it's not connected
• _lowLevelOpenUnit function raises
• The object is destroyed, calling __del__, calling close(), calling _lowLevelCloseUnit
• _lowLevelCloseUnit raises too because c_handle.value is wrong (for a good reason) since the beginning.
  And we ended up with two raised exception for one information (we're no connected), and that generates ignored exceptions in my code. The fix allow for the last raised exception to be truly ignored, and the AttributeError one to never show up.