Skip to content

Use weakref finalize to close visa connections#5341

Merged
jenshnielsen merged 12 commits intomicrosoft:masterfrom
jenshnielsen:shutdown_visa
Sep 12, 2023
Merged

Use weakref finalize to close visa connections#5341
jenshnielsen merged 12 commits intomicrosoft:masterfrom
jenshnielsen:shutdown_visa

Conversation

@jenshnielsen
Copy link
Copy Markdown
Collaborator

@jenshnielsen jenshnielsen commented Sep 3, 2023

Partial fix for #3774 it's tricky to do this generally since the finalizer cannot take a reference to the instrument it self and therefore also not a method on this such as self.close.

For visa instruments this is possible to implement since we only need a reference to the visa_handle to close the connection. This could also be implemented more generally if we created a handle/closer class on all instruments.

This also adds tests that confirms that visa instruments are closed once the instrument is out of scope and gc'ed.

Closes #5356 by closing the resource_manager in close if the backend is sim. Since this only applies to simulated instruments we don't add it to the finalizer but assume as always that a test must cleanup after it self.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 3, 2023

Codecov Report

Merging #5341 (a033320) into master (43560ec) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5341      +/-   ##
==========================================
+ Coverage   67.62%   67.63%   +0.01%     
==========================================
  Files         360      360              
  Lines       30059    30069      +10     
==========================================
+ Hits        20328    20338      +10     
  Misses       9731     9731              

@jenshnielsen jenshnielsen marked this pull request as ready for review September 5, 2023 08:52
Copy link
Copy Markdown
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

and a newsfragment? :)

@jenshnielsen jenshnielsen added this pull request to the merge queue Sep 12, 2023
Merged via the queue into microsoft:master with commit 692a2e6 Sep 12, 2023
@jenshnielsen jenshnielsen deleted the shutdown_visa branch September 12, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pyvisa sim instruments are not cleanup on close.

2 participants