nion-software / nionswift-instrumentation-kit

Base classes for Nion Swift STEM microscope instrumentation
GNU General Public License v3.0
1 stars 12 forks source link

YA: fix broken scan dialog #157

Closed yvesauad closed 1 year ago

yvesauad commented 1 year ago

Minor typo. This has been broken since 0.21.X. The method open_configuration_interface should belong to ScanDevice, as also used in usim scandevice example.

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

cmeyer commented 1 year ago

The scan hardware source contains a method to dispatch the configuration dialog to the scan settings object. That occurs here:

https://github.com/nion-software/nionswift-instrumentation-kit/blob/aa05886e360248c687ce2ff20e1b62323ca7ed34/nion/instrumentation/scan_base.py#L2015

The scan settings allow a separation between the device and configuration. You can see an example of how scan settings are used in the simulator code here:

https://github.com/nion-software/nionswift-usim/blob/87f4c7a797dc45d202549a3d90b5555f7743976d/nionswift_plugin/usim/ScanDevice.py#L534

When using ScanSettings, you can pass an additional parameter open_configuration_dialog_fn to supply the configuration dialog. Here is the interface for ScanSettings:

https://github.com/nion-software/nionswift-instrumentation-kit/blob/aa05886e360248c687ce2ff20e1b62323ca7ed34/nion/instrumentation/scan_base.py#L944

Finally, please note that all of this code is still somewhat in flux as I make the final changes to accommodate secondary scan devices. I'm hoping it will not change much, but there is still a small chance another interface change will be required.

yvesauad commented 1 year ago

Hi Chris, thanks for answering!

I am using the class ScanSettings now. I really your idea of a functional usim at all versions, so when in doubt I just check usim and change my code accordingly. It would help us if you create a minimal example in the USIM for a working configuration dialog.

I suspect there is a small missing detail in your ScanSetting open_configuration parameter approach. To exemplify, I did a test inhering your scansettings and slightly changing open_configuration_interface

class ScanSettings(scan_base.ScanSettings): def init(self, scan_modes, frame_parameters_factory, current_settings_index = 0, record_settings_index = 0, open_configuration_dialog_fn = None) -> None: super(ScanSettings, self).init(scan_modes, frame_parameters_factory, current_settings_index, record_settings_index, open_configuration_dialog_fn) def open_configuration_interface(self, api_broker: typing.Any) -> None: if callable(self.__open_configuration_dialog_fn): self.__open_configuration_dialog_fn(api_broker)

I only added api_broker as an argument of my open_configuration_dialog. The rest is identical.

Then in my ScanModule class, I created a ScanSettings such as

self.settings = ScanSettings(scan_modes, lambda d: scan_base.ScanFrameParameters(d), 0, 2, open_configuration_dialog_fn=show_configuration_dialog)

In which my function show_configuration_dialog is

def show_configuration_dialog(api_broker) -> None: """Open settings dialog, if any.""" api = api_broker.get_api(version="1", ui_version="1") document_controller = api.application.document_controllers[0]._document_controller myConfig = ConfigDialog(document_controller)

This is working greatly. If I understand correctly, you need to pass api_broker to the callable argument function, right?

cheers,

Yves

On 4/14/23 22:44, Chris Meyer wrote:

The scan hardware source contains a method to dispatch the configuration dialog to the scan settings object. That occurs here:

https://github.com/nion-software/nionswift-instrumentation-kit/blob/aa05886e360248c687ce2ff20e1b62323ca7ed34/nion/instrumentation/scan_base.py#L2015

The scan settings allow a separation between the device and configuration. You can see an example of how scan settings are used in the simulator code here:

https://github.com/nion-software/nionswift-usim/blob/87f4c7a797dc45d202549a3d90b5555f7743976d/nionswift_plugin/usim/ScanDevice.py#L534

When using |ScanSettings|, you can pass an additional parameter |open_configuration_dialog_fn| to supply the configuration dialog. Here is the interface for |ScanSettings|:

https://github.com/nion-software/nionswift-instrumentation-kit/blob/aa05886e360248c687ce2ff20e1b62323ca7ed34/nion/instrumentation/scan_base.py#L944

Finally, please note that all of this code is still somewhat in flux as I make the final changes to accommodate secondary scan devices. I'm hoping it will not change much, but there is still a small chance another interface change will be required.

— Reply to this email directly, view it on GitHub https://github.com/nion-software/nionswift-instrumentation-kit/pull/157#issuecomment-1509235525, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6U66R4HU4QW3VKWPWTKQTXBGZMRANCNFSM6AAAAAAW5DJRCI. You are receiving this because you authored the thread.Message ID: @.***>

yvesauad commented 1 year ago

My bad on the formatting. I directly reply in my email. In short, this is currently ScanSettings:

def open_configuration_interface(self, api_broker: typing.Any) -> None:
        if callable(self.__open_configuration_dialog_fn):
            self.__open_configuration_dialog_fn()

Shouldnt we pass the api_broker to the open_configuration_dialog_fn? If we do this:

def open_configuration_interface(self, api_broker: typing.Any) -> None:
        if callable(self.__open_configuration_dialog_fn):
            self.__open_configuration_dialog_fn(api_broker)

everything works greatly :) cheers

cmeyer commented 1 year ago

I fixed this problem somewhat differently. The goal is to eliminate the need for a device to provide a dialog and instead have the "settings" provide the configuration dialog. The settings object can still call the device to provide the dialog if it wants to, but it may also implement it separately.

So there was a bug in that the api-broker was not passed to the callback function passed to settings. I fixed that here: 4746e53c620051779c5fb9048a50ce886aa30dca

If you want to call a device method to do the configuration dialog, you can pass device.open_configuration_interface to scan_base.ScanSettings. The commit above changes the callback passed to ScanSettings to take an api_broker parameter; but also requires you to define **kwargs for future expansion.

This is a bit complicated, so if you want more help, please let me know. I think you can make your ScanSettings line look like this:

self.settings = ScanSettings(scan_modes, lambda d: scan_base.ScanFrameParameters(d), 0, 2, open_configuration_dialog_fn=device.show_configuration_dialog)

and your show_configuration_dialog function in your Device object should have the signature:

def open_configuration_interface(self, api_broker: typing.Any, **kwargs: typing.Any) -> None: