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.

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

Add nplc to gui #5

Merged
merged 3 commits into from
Apr 22, 2024
Merged

Add nplc to gui #5

merged 3 commits into from
Apr 22, 2024

Conversation

AlexanderWells-diamond
Copy link
Contributor

This PR adds the "NPLC" configuration item to the main GUI. Previously this value was hard-coded as "50", hence the default value.

As this was previously set during instrument initialization and was never expected to change, this PR required an unexpectedly large amount of restructuring. Main changes include:

  • Adding "enabled" to the InstrumentConfig, so that the internal data model now mirrors the GUI's checkboxes
  • Refactored _init_instrument into sub-functions, to allow re-use for set_nplc.
  • Added a bunch of logging and error message handling, so more errors are now visible on the GUI and/or more messages are sent to the logger.

image

This commit means that we will no longer attempt to send messages to
instruments that have never been configured / are disabled entirely.
Previously, when using the default NPLC of 50, the timeout wouldn't have
been modified and the default timeout is too short.

Also add tests for all NPLC setting code
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Attention: Patch coverage is 63.93443% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 60.36%. Comparing base (fa06fac) to head (4875dda).
Report is 2 commits behind head on main.

Files Patch % Lines
src/psc_datalogger/gui.py 0.00% 13 Missing ⚠️
src/psc_datalogger/connection.py 81.25% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #5      +/-   ##
==========================================
+ Coverage   59.16%   60.36%   +1.19%     
==========================================
  Files           7        7              
  Lines         409      444      +35     
==========================================
+ Hits          242      268      +26     
- Misses        167      176       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlexanderWells-diamond AlexanderWells-diamond merged commit c022b0b into main Apr 22, 2024
24 checks passed
@AlexanderWells-diamond AlexanderWells-diamond deleted the add_nplc_to_gui branch April 22, 2024 09:38
@AlexanderWells-diamond AlexanderWells-diamond restored the add_nplc_to_gui branch July 19, 2024 07:53
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.

1 participant