Skip to content

Hla 1124 improve offset zeroing and restoration - #94

Merged
ptsOSL merged 14 commits into
mainfrom
hla-1124-improve-offset-zeroing-and-restoration
Jul 30, 2026
Merged

Hla 1124 improve offset zeroing and restoration#94
ptsOSL merged 14 commits into
mainfrom
hla-1124-improve-offset-zeroing-and-restoration

Conversation

@ptsOSL

@ptsOSL ptsOSL commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator
  • We now save all 3 bpm offsets to file and automatically zero and then later restore BCD and golden offsets.
  • We now pass "FULL_SAVE_LOCATION" to the machines config dictionary. This is needed so that the algorithms can load in the BBA offsets file to calculate the change in BBA offsets.
  • Tidy up offset handling in GUI and CLI to remove references to golden offsets and make the interface generic to any offsets file.
  • Remove redundant code and fix bug in CLI.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 22.72727% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.20%. Comparing base (0ab37fb) to head (32142c0).

Files with missing lines Patch % Lines
src/dls_bba/machine.py 14.54% 47 Missing ⚠️
src/dls_bba/gui.py 33.33% 6 Missing ⚠️
src/dls_bba/worker.py 40.00% 6 Missing ⚠️
src/dls_bba/__main__.py 40.00% 3 Missing ⚠️
src/dls_bba/algorithm.py 25.00% 3 Missing ⚠️
src/dls_bba/common.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
- Coverage   51.57%   51.20%   -0.37%     
==========================================
  Files          20       20              
  Lines        2257     2277      +20     
==========================================
+ Hits         1164     1166       +2     
- Misses       1093     1111      +18     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ptsOSL
ptsOSL force-pushed the hla-1124-improve-offset-zeroing-and-restoration branch from fb30190 to bddfa1a Compare July 24, 2026 15:13
ptsOSL added 2 commits July 24, 2026 15:31
If foldername is somehow None at this point, then we must have
failed to create a directory. In this case, using its parent directory as
a fallback doesnt make much sense. We have no way of knowing if the parent
exists.
@ptsOSL
ptsOSL force-pushed the hla-1124-improve-offset-zeroing-and-restoration branch from bddfa1a to 06d4b5c Compare July 24, 2026 15:34
@ptsOSL
ptsOSL force-pushed the hla-1124-improve-offset-zeroing-and-restoration branch from 06d4b5c to 3949d0a Compare July 24, 2026 15:55
@ptsOSL ptsOSL changed the title Draft: Hla 1124 improve offset zeroing and restoration Hla 1124 improve offset zeroing and restoration Jul 24, 2026
And some comments/docstring improvements.
@ptsOSL
ptsOSL force-pushed the hla-1124-improve-offset-zeroing-and-restoration branch from 15e1bcb to 030e84b Compare July 24, 2026 17:18

@MJGaughran MJGaughran left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have to admit I'm mostly confused by this PR, and it's probably due to the history of this code.

The constant use of "folder_name" and "file_path" as variables is not great.

Comment thread src/dls_bba/__main__.py
Comment thread src/dls_bba/common.py Outdated
Comment thread src/dls_bba/worker.py
self.machine = machine
else:
self.machine = Machine(extra_config_files, additional_options)
folder_path = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If foldername is somehow None at this point, then we must have
failed to create a directory. In this case, using its parent directory as
a fallback doesnt make much sense. We have no way of knowing if the parent
exists.

The above is the git commit message that attempts to explain this change, but I don't quite follow it. It might be easier to go over it in person.

It looks like the code change implies that:

  • Previously, it would use self.machine.config["SAVE_LOCATION"] as a fallback
  • Now, it uses os.getcwd()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We improved this and the fallback is now config["SAVE_LOCATION"] again. If this is also undefined or an invalid path, then there will likely be an uncaught exception. But that is out of the scope of this PR

ptsOSL added 5 commits July 29, 2026 15:26
If no folder_path is passed in we now try to get the SAVE_LOCATION variable
from a config file/dictionary. To get SAVE_LOCATION we must first create
the machine. But to set FULL_SAVE_LOCATION we must first create the
save location which requires SAVE_LOCATION. This means we must first
create the machine, then the save location and then update the machine
with the full save location.
@ptsOSL
ptsOSL force-pushed the hla-1124-improve-offset-zeroing-and-restoration branch from d0a1614 to 32142c0 Compare July 29, 2026 17:03
@ptsOSL
ptsOSL merged commit 43999e1 into main Jul 30, 2026
8 of 10 checks passed
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.

2 participants