Settings load fixes, a clearer position read error, and dead code removal

src/main/java/org/micromanager/lightsheetmanager/gui/tabs/AcquisitionTab.java

@@ -125,6 +125,8 @@ private void createUserInterface() {
         pnlMultiPositions_ = new PositionPanel(model_, cbxUseMultiplePositions_);
         // disable elements based on settings
         pnlMultiPositions_.setPanelEnabled(settings.isUsingMultiplePositions());
+        // the playlist opens the same grid frame as the positions panel
+        acqTableFrame_.setXYZGridFrame(pnlMultiPositions_.getXYZGridFrame());
 
         pnlSaveData_ = new SavePanel(model_, frame_);
         pnlCameras_ = new CameraPanel(model_);

src/main/java/org/micromanager/lightsheetmanager/gui/tabs/TabPanel.java

@@ -106,7 +106,6 @@ private void createUserInterface() {
         } else {
             tabbedPane_.setSelectedIndex(0); // Navigation Tab
         }
-        tabbedPane_.setSelectedIndex(numImagingPaths + 1);
 
         // add ui elements to the panel
         add(tabbedPane_, "growx, growy");

src/main/java/org/micromanager/lightsheetmanager/model/SettingsAdapter.java

@@ -121,7 +121,7 @@ public SettingsAdapter(final LightSheetManager model) {
                 value -> model_.acquisitions().settingsBuilder()
                         .autofocusBuilder().enabled((boolean) value));
 
-        // useMovementCorrection - not used, old planar correction feature
+        // useMovementCorrection - not used, motion correction (follows sample drift between time points)
         // acquireBothCamerasSimultaneously - not used, uses camera order to specify which cameras are active
         // numSides - not used, SCAPE microscopes only have a single view
         // firstSideIsA - not used, SCAPE microscopes only have a single view

src/main/java/org/micromanager/lightsheetmanager/model/UserSettings.java

@@ -8,11 +8,11 @@
 import org.micromanager.lightsheetmanager.api.data.GeometryType;
 import org.micromanager.lightsheetmanager.api.internal.ScapeAcquisitionSettings;
 import org.micromanager.lightsheetmanager.gui.components.SettingsListener;
+import org.micromanager.lightsheetmanager.model.utils.JsonUtils;
 import org.micromanager.propertymap.MutablePropertyMapView;
 
 import javax.swing.SwingUtilities;
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -171,90 +171,27 @@ public void save() {
     private Optional<JSONObject> validateUserSettings(final String loadedSettings) {
         // create default settings from builder
         final String defaultSettings = ScapeAcquisitionSettings.builder().build().toJson();
-        // validate JSON strings and count the number of keys
-        int numLoadedKeys;
-        int numDefaultKeys;
+        // validate JSON strings
         JSONObject loadedJson;
         JSONObject defaultJson;
         try {
             loadedJson = new JSONObject(loadedSettings);
             defaultJson = new JSONObject(defaultSettings);
-            numLoadedKeys = countKeysJson(loadedJson);
-            numDefaultKeys = countKeysJson(defaultJson);
         } catch (JSONException e) {
             model_.studio().logs().showError("could not validate the JSON data.");
             return Optional.empty();
         }
-        // different number of keys => merge loaded settings with default settings
-        if (numLoadedKeys != numDefaultKeys) {
-            try {
-                mergeSettingsJson(defaultJson, loadedJson);
-            } catch (JSONException e) {
-                model_.studio().logs().showError("could not merge new default settings into loaded settings.");
-                return Optional.empty();
-            }
+        // add every default setting the loaded settings lack, including any whose key was renamed
+        try {
+            JsonUtils.addMissingKeys(defaultJson, loadedJson, MAX_RECURSION_DEPTH_JSON,
+                    message -> model_.studio().logs().logMessage("UserSettings: " + message));
+        } catch (JSONException e) {
+            model_.studio().logs().showError("could not merge new default settings into loaded settings.");
+            return Optional.empty();
         }
         return Optional.of(loadedJson);
     }
 
-    // Overloaded method to give mergeSettingsJson a default parameter.
-    private void mergeSettingsJson(JSONObject defaultJson, JSONObject loadedJson) throws JSONException {
-        mergeSettingsJson(defaultJson, loadedJson, 0);
-    }
-
-    private void mergeSettingsJson(
-            JSONObject defaultJson, JSONObject loadedJson, final int level) throws JSONException {
-
-        // bail out if settings data is nested too deep
-        if (level > MAX_RECURSION_DEPTH_JSON) {
-            model_.studio().logs().logMessage("UserSettings: recursion too deep, increase max level.");
-            return; // early exit => recursion too deep
-        }
-
-        // for every key in the default settings, check to make sure the loaded settings has that key
-        Iterator<String> keys = defaultJson.keys();
-        while (keys.hasNext()) {
-            final String key = keys.next();
-            final Object value = defaultJson.get(key);
-            // if the loaded settings are missing the key then add it
-            if (!loadedJson.has(key)) {
-                loadedJson.put(key, value);
-                model_.studio().logs().logMessage(
-                        "UserSettings: Added key \"" + key + "\" to the loaded settings.");
-            }
-            // recursively call on sub-objects of type JSONObject
-            if (value instanceof JSONObject) {
-                JSONObject subDefaultJson = (JSONObject)value;
-                JSONObject subLoadedJson = (JSONObject)loadedJson.get(key);
-                if (subLoadedJson.length() != subDefaultJson.length()) {
-                    mergeSettingsJson(subDefaultJson, subLoadedJson, level+1);
-                    loadedJson.put(key, subLoadedJson);
-                }
-            }
-        }
-    }
-
-    /**
-     * Return the number of keys in the {@code JSONObject} object and
-     * all internal {@code JSONObject} objects.
-     *
-     * @param obj the root {@code JSONObject} object
-     * @return the number of keys in all objects
-     * @throws JSONException error getting value from key from object
-     */
-    private int countKeysJson(final JSONObject obj) throws JSONException {
-        int numKeys = obj.length();
-        Iterator<String> keys = obj.keys();
-        while (keys.hasNext()) {
-            final String key = keys.next();
-            final Object value = obj.get(key);
-            if (value instanceof JSONObject) {
-                numKeys += ((JSONObject)value).length();
-            }
-        }
-        return numKeys;
-    }
-
     /**
      * Adds a listener that will update when the settings are loaded.
      *

src/main/java/org/micromanager/lightsheetmanager/model/acquisitions/AcquisitionEngineDispim.java

@@ -255,12 +255,6 @@ boolean run() {
         sink.setDatastore(datastore_);
         sink.setPipeline(curPipeline_);
 
-        studio_.events().registerForEvents(this);
-        // commented because this is prob specific to MM MDAs
-//        studio_.events().post(new DefaultAcquisitionStartedEvent(datastore_, this,
-//              acquisitionSettings));
-
-
         // TODO if position time ordering ever implemented, this should be reactivated and the
         //  timelapse hook copied from org.micromanager.acquisition.internal.acqengjcompat.AcqEngJAdapter
 //        if (sequenceSettings_.acqOrderMode() == AcqOrderMode.POS_TIME_CHANNEL_SLICE
@@ -577,9 +571,6 @@ void finish() {
             }
         }
 
-        // unregister to stop ghost events
-        studio_.events().unregisterForEvents(this);
-
         // start polling for navigation panel
         if (isPolling_) {
             studio_.logs().logMessage("started position polling after acquisition");

src/main/java/org/micromanager/lightsheetmanager/model/acquisitions/AcquisitionEngineScape.java

@@ -361,11 +361,6 @@ boolean run() {
             return false;
         }
 
-        studio_.events().registerForEvents(this);
-        // commented because this is prob specific to MM MDAs
-//        studio_.events().post(new DefaultAcquisitionStartedEvent(datastore_, this,
-//              acquisitionSettings));
-
         final String[] cameraNames = resolveCameraNames(settings, demoMode);
 
         // read once, before the acquisition starts, so every channel's offset has the same origin
@@ -1005,12 +1000,13 @@ private void submitEvents(final ScapeAcquisitionSettings settings, final String[
                             // AcqEngJ split the merge and start one more camera sequence than the
                             // controller was armed to deliver, which then waited for frames that
                             // never arrived.
-                            // VOLUME_HW reaches this branch too and is filed wrong when it does:
+                            // VOLUME_HW with more than one channel is filed wrong on this branch:
                             // it switches channel once per volume, so it needs the channel axis
                             // outside the z axis rather than innermost. The channel panel steers
                             // users away from it but does not write the corrected mode back to the
                             // settings, so a stored profile or an API caller still arrives here.
-                            // Nothing refuses it on this path yet.
+                            // doHardwareCalculations refuses it on PLogic runs; demo and NIDAQ
+                            // runs skip that check and still reach this branch.
                             currentAcquisition_.submitEventIterator(
                                     LightSheetEventAdapter.createChannelPerSliceAcqEvents(
                                             baseEvent.copy(), settings, cameraNames,
@@ -1393,9 +1389,6 @@ void finish() {
         }
         originalChannelPreset_ = null;
 
-        // unregister to stop ghost events
-        studio_.events().unregisterForEvents(this);
-
         // start polling for navigation panel
         if (isPolling_) {
             studio_.logs().logMessage("started position polling after acquisition");

src/main/java/org/micromanager/lightsheetmanager/model/acquisitions/LightSheetEventAdapter.java

@@ -176,12 +176,12 @@ public static Iterator<AcquisitionEvent> createSingleChannelVolumeAcqEvents(
             Function<AcquisitionEvent, AcquisitionEvent> eventMonitor,
             int channelIndex, ChannelSpec channel, int numUsedChannels) {
 
-        // Bake this one channel's config into the base event (what channels().next() does per channel).
+        // Bake this one channel's config into the base event.
         baseEvent.setConfigGroup(channel.getGroup());
         baseEvent.setConfigPreset(channel.getName());
         baseEvent.setChannelName(Integer.toString(channelIndex));
 
-        // Channel z-offset: mirror channels(), apply the offset to the current stage/z position.
+        // Channel z-offset: apply the offset to the current stage/z position.
         double zPos;
         if (baseEvent.getZPosition() == null) {
             try {
@@ -265,14 +265,14 @@ public static Iterator<AcquisitionEvent> createChannelPerSliceAcqEvents(
     }
 
     /**
-     * Bake one channel's preset and z-offset onto the base event, as {@link #channels} stamps them
-     * per event on the software path.
+     * Bake one channel's preset and z-offset onto the base event, for a run with a single channel
+     * in use.
      */
     private static void applySingleChannel(AcquisitionEvent baseEvent, ChannelSpec channel) {
         baseEvent.setConfigGroup(channel.getGroup());
         baseEvent.setConfigPreset(channel.getName());
 
-        // Channel z-offset: mirror channels(), apply the offset to the current stage/z position.
+        // Channel z-offset: apply the offset to the current stage/z position.
         double zPos;
         if (baseEvent.getZPosition() == null) {
             try {
@@ -362,7 +362,7 @@ public AcquisitionEvent next() {
     /**
      * Fan an event out over the cameras, deriving the channel-axis base from the event.
      * <p>
-     * Used by the {@link #channels}-composed factories.
+     * Used by the factories that run with channels disabled, where the base is 0.
      */
     public static Function<AcquisitionEvent, Iterator<AcquisitionEvent>> cameras(String[] cameraDeviceNames) {
         return cameras(cameraDeviceNames, null);
@@ -376,14 +376,14 @@ public static Function<AcquisitionEvent, Iterator<AcquisitionEvent>> cameras(Str
      * channel index and the camera index share one axis, and the coordinate written here is the combined
      * slot {@code channelIndex + cameraIndex * numChannels} that {@code addMMSummaryMetadata} names and
      * sizes the datastore for. Do not treat a value already present on that axis as a camera base unless
-     * you put it there: on the {@code channels()}-composed paths it is the raw channel index written by
-     * {@code setChannelName}.
+     * you put it there: {@link #createSingleChannelVolumeAcqEvents} writes the raw channel index there
+     * with {@code setChannelName}, and passes its base explicitly for that reason.
      * <p>
      * This overload steps one slot per camera, which is only the whole coordinate where a single
      * channel is in play; a multi-channel caller wants the stride overload below.
      *
      * @param channelAxisBase the channel-axis base for this fan-out, or {@code null} to derive it from
-     *                        the event: the channel index {@code channels()} left on the axis
+     *                        the event: the value already on the axis, or 0 when there is none
      */
     public static Function<AcquisitionEvent, Iterator<AcquisitionEvent>> cameras(
             String[] cameraDeviceNames, Integer channelAxisBase) {
@@ -428,10 +428,10 @@ public AcquisitionEvent next() {
                     // Caller-supplied base (createSingleChannelVolumeAcqEvents, per-channel path).
                     baseIndex = channelAxisBase;
                 } else {
-                    // The base is the channel index channels() wrote onto this axis via
-                    // setChannelName (absent => 0 when channels are disabled). Read it off the event
-                    // rather than from shared state: run() submits one iterator per position per
-                    // timepoint eagerly, and AcqEngJ consumes them lazily and interleaved.
+                    // The base is the value already on this axis (absent => 0, as when channels
+                    // are disabled). Read it off the event rather than from shared state: run()
+                    // submits one iterator per position per timepoint eagerly, and AcqEngJ
+                    // consumes them lazily and interleaved.
                     Object position = event.getAxisPosition(CAMERA_AXIS);
                     int parsed = 0;
                     if (position != null) {
@@ -502,49 +502,6 @@ public AcquisitionEvent next() {
         };
     }
 
-    /**
-     * Make an iterator for events for each active channel
-     *
-     * @param channelList the list of channels to iterate over
-     * @return
-     */
-    public static Function<AcquisitionEvent, Iterator<AcquisitionEvent>> channels(
-            ChannelSpec[] channelList) {
-        return (AcquisitionEvent event) -> new Iterator<>() {
-            int index = 0;
-
-            @Override
-            public boolean hasNext() {
-                return index < channelList.length;
-            }
-
-            @Override
-            public AcquisitionEvent next() {
-                AcquisitionEvent channelEvent = event.copy();
-                channelEvent.setConfigGroup(channelList[index].getGroup());
-                channelEvent.setConfigPreset(channelList[index].getName());
-                channelEvent.setChannelName(Integer.toString(index));
-
-                double zPos;
-                if (channelEvent.getZPosition() == null) {
-                    try {
-                        zPos = Engine.getCore().getPosition() + channelList[index].getOffset();
-                    } catch (Exception e) {
-                        throw new RuntimeException(e);
-                    }
-                } else {
-                    zPos = channelEvent.getZPosition() + channelList[index].getOffset();
-                }
-                channelEvent.setZ(channelEvent.getZIndex(), zPos);
-
-                // TODO: do channels have different exposures?
-//               channelEvent.setExposure(channelList.get(index).exposure());
-                index++;
-                return channelEvent;
-            }
-        };
-    }
-
     /**
      * Iterate over an arbitrary list of positions. Adds in position indices to
      * the axes that assume the order in the list provided correspond to the

src/main/java/org/micromanager/lightsheetmanager/model/positions/PositionUpdater.java

@@ -126,7 +126,8 @@ public void updatePositions() {
                positions_.put(device, model_.core().getGalvoPosition(deviceName));
             }
          } catch (Exception e) {
-            model_.studio().logs().logError("Error in updatePositions()");
+            model_.studio().logs().logError("Position polling could not read " + device
+                  + " (" + deviceName + "): " + e.getMessage());
          }
       }
    }