WasatchPhotonics / ENLIGHTEN

Open-source spectroscopy application for controlling and taking measurements from Wasatch Photonics spectrometers.
https://wasatchphotonics.com/product-category/software/
MIT License
3 stars 6 forks source link

Sb persist sg #328

Closed samiebee43 closed 10 months ago

samiebee43 commented 10 months ago

What has changed

Adjustments to be more compliant with style guide #240

Gain, Integration Time, and Boxcar had the following change of pattern for updating persistent parameters. The result is one less state-to-state copying.

image

Gain had a rewrite of its load from persistence such that the code is structured to match the searchpath specified in the issue above.

Todo

samiebee43 commented 10 months ago

Highlighted diff: Configuration.py

This is a significant portion of the diff that I'd like to highlight. No longer in Configuration's save_file routine do we last-minute copy from one memory location to another. ~This also has the clue that state.integration_time_ms et al will need to have all of its references replaced with self.ctl.config.get(...)~ (thus config can be used for inter-session and same-session persistence)

Configuration.py

...
def save_file(...):
    ...
-    self.set(sn, "integration_time_ms", state.integration_time_ms)
-    self.set(sn, "boxcar_half_width", state.boxcar_half_width)
-    self.set(sn, "gain_db", state.gain_db)
    ...
samiebee43 commented 10 months ago

Scan for state references

Unfortunately there are great number of references to state.integration_time_ms et al. It's enough to oversize the scope of this pull request. So instead I put back (coined "re-add") the saves to multispec state. Still we want to change config's memory model immediately instead of waiting for the shutdown event.

Now the approach for persistent variable modification is to write out each stage and clearly comment them:

# save integration time to application state
self.multispec.set_state("integration_time_ms", now_ms)

# persist integration time in .ini
self.ctl.config.set(self.ctl.multispec.current_spectrometer().settings.eeprom.serial_number, "integration_time_ms", ms)

# send integration time change to hardware
spec.change_device_setting("integration_time_ms", now_ms)

Note that IntegrationTimeFeature and BoxcarFeature still have their self.multispec variable, but GainDBFeature is already switched to self.ctl.multispec.

mzieg commented 10 months ago

I haven't digested all of this, but if you remove the calls to Multispec, are you eliminating the feature where multiple spectrometers could be controlled at once?

samiebee43 commented 10 months ago

I haven't digested all of this, but if you remove the calls to Multispec, are you eliminating the feature where multiple spectrometers could be controlled at once?

No. Nothing changes in terms of functionality.

samiebee43 commented 10 months ago

This closes the basic requirements of #240, but we should copy it's contents to Discussion or Wiki since it can be used for broader improvements to persistent variables' lifecycle--can be applied to anything that uses self.ctl.config get or set.