fiji_scripts: return error messages instead of undef when importing FIJI Settings

This allows to present the user with a meaningful message in the main window.

FIJI/Settings.pm

@@ -26,6 +26,8 @@ use FIJI qw(:all);
 #        read from file. Existing values will not be validated.
 #        Superfluous elements not present in the existing hash will be
 #        removed, e.g. FIUs.
+# \returns The new settings instance or a string describing the reason
+#          why it could not be created.
 sub new ($;$$) {
   my $logger = get_logger();
   my ($class, $phase, $fiji_ini_file, $existing_settings) = @_;
@@ -38,8 +40,9 @@ sub new ($;$$) {
     _set_defaults(DESIGNMAP, $fiji_settings_ref->{'design'}, $phase);
     $fiji_settings_ref = bless($fiji_settings_ref, $class);
     if (!ref($fiji_settings_ref) || !UNIVERSAL::can($fiji_settings_ref,'can')) {
-      $logger->error("Could not bless FIJI::Settings class from \"$fiji_ini_file\".");
-      return undef;
+      my $msg = "Could not bless FIJI::Settings class from \"$fiji_ini_file\".";
+      $logger->error($msg);
+      return $msg;
     }
   } else {
     $fiji_settings_ref = _clone($existing_settings);
@@ -47,8 +50,8 @@ sub new ($;$$) {
 
   if (defined($fiji_ini_file)) {
     $fiji_settings_ref = read_settingsfile($phase, $fiji_ini_file, $fiji_settings_ref);
-    if (!defined($fiji_settings_ref)) {
-      return undef;
+    if (!ref($fiji_settings_ref)) {
+      return $fiji_settings_ref; # actually an error message
     }
     if (defined($existing_settings)) {
       return $existing_settings;
@@ -169,27 +172,31 @@ sub read_settingsfile {
   my $fiji_ini;
   eval { $fiji_ini = new Config::Simple($fiji_ini_file) }; # pesky library tries to die on syntax errors
   if (!defined($fiji_ini)) {
-    $logger->error("Could not read config file \"$fiji_ini_file\": " . (defined($@) ? $@ : Config::Simple->error()));
-    return undef;
+    my $msg = "Could not read config file \"$fiji_ini_file\": " . (defined($@) ? $@ : Config::Simple->error());
+    $logger->error($msg);
+    return $msg;
   }
 
   my $fiji_design_cfg = $fiji_ini->get_block("CONSTS");
   if (!(%$fiji_design_cfg)) {
-    $logger->error("Could not fetch CONSTS block from config file \"$fiji_ini_file\"");
-    return undef;
+    my $msg = "Could not fetch CONSTS block from config file \"$fiji_ini_file\"";
+    $logger->error($msg);
+    return $msg;
   }
 
   my $fiji_design = _rename_import(DESIGNMAP, $fiji_design_cfg);
   if (!defined($fiji_design)) {
-    $logger->error("Design constants do not match the FIJI Settings naming scheme.");
-    return undef;
+    my $msg = "Design constants do not match the FIJI Settings naming scheme.";
+    $logger->error($msg);
+    return $msg;
   }
 
   my $fiji_settings_ref;
   if (defined($existing_settings)) {
     if (!blessed($existing_settings) || !$existing_settings->isa("FIJI::Settings")) {
-      $logger->error("Given settings are not of type FIJI::Settings.");
-      return undef;
+      my $msg ="Given settings are not of type FIJI::Settings.";
+      $logger->error($msg);
+      return $msg;
     }
     # overwrite existing Design Constants
     foreach my $k (keys($fiji_design)) {
@@ -207,9 +214,9 @@ sub read_settingsfile {
 
   # sanitize and validate read design constants
   $fiji_design = _sanitize_design($fiji_design, $phase);
-  if (!defined($fiji_design)) {
-    $logger->error("Design constants in FIJI Settings invalid");
-    return undef;
+  if (!ref($fiji_design)) {
+    $logger->error($fiji_design);
+    return $fiji_design;
   }
   $fiji_settings_ref->{'design'} = $fiji_design;
 
@@ -222,8 +229,9 @@ sub read_settingsfile {
     }
     my $fiji_fiu = _rename_import(FIUMAP, $fiji_fiu_cfg);
     if (!defined($fiji_design)) {
-      $logger->error("FIU constants of $fiu_name do not match the FIJI Settings naming scheme.");
-      return undef;
+      my $msg = "FIU constants of $fiu_name do not match the FIJI Settings naming scheme.";
+      $logger->error($msg);
+      return $msg;
     }
     my $tmp_fiu = {};
     _set_defaults(FIUMAP, $tmp_fiu, $phase);
@@ -245,8 +253,9 @@ sub read_settingsfile {
     }
     $fiji_fiu = _sanitize_fiu($fiji_fiu, $phase);
     if (!defined($fiji_fiu)) {
-      $logger->error("Constants for $fiu_name in FIJI Settings are invalid");
-      return undef;
+      my $msg = "Constants for $fiu_name in FIJI Settings are invalid";
+      $logger->error($msg);
+      return $msg;
     }
     @{$fiji_settings_ref->{'FIUs'}}[$fiu_num] = $fiji_fiu;
     $fiu_num++;
@@ -261,8 +270,9 @@ sub read_settingsfile {
   # FIU_NUM is optional in the Settings file... if it was set check that
   # it corresponds to the number of FIU<number> blocks.
   if (defined($fiji_design->{'FIU_NUM'}) && $fiji_design->{'FIU_NUM'} != $fiu_num) {
-      $logger->error(FIU_NUM->{'ini_name'} . " does not match the numbers of FIU blocks found.");
-      return undef;
+      my $msg = FIU_NUM->{'ini_name'} . " does not match the numbers of FIU blocks found.";
+      $logger->error($msg);
+      return $msg;
   } else {
     $fiji_design->{'FIU_NUM'} = $fiu_num; # assume the best if FIU_NUM constant is not given
   }
@@ -454,14 +464,15 @@ sub _sanitize_fiu ($;$) {
   my $logger = get_logger();
   my ($fiu_ref, $phase) = @_;
   if (ref($fiu_ref) ne 'HASH') {
-    $logger->error("Parameter is not a reference to a hash (containing FIU constants).");
-    return undef;
+    my $msg = "Parameter is not a reference to a hash (containing FIU constants).";
+    $logger->error($msg);
+    return $msg;
   }
 
   $fiu_ref = _validate_hashmap(FIUMAP, $fiu_ref, $phase);
-  if (!defined($fiu_ref)) {
+  if (!ref($fiu_ref)) {
       $logger->error("Could not validate Design Constants.");
-      return undef;
+      return $fiu_ref;
   }
 
   return $fiu_ref;
@@ -483,8 +494,9 @@ sub _validate_hashmap ($$;$) {
     # mark constant key as done
     @map_keys = grep {$_ ne $entry_key} @map_keys;
     if (!validate_value($map_ref, $entry_key, \$consts_ref->{$entry_key})) {
-      $logger->error(sprintf("%s = %s is invalid.", $entry_key, !defined($consts_ref->{$entry_key}) ? "<undef>" : $consts_ref->{$entry_key}));
-      return undef;
+      my $msg = sprintf("%s = %s is invalid.", $entry_key, !defined($consts_ref->{$entry_key}) ? "<undef>" : $consts_ref->{$entry_key});
+      $logger->error($msg);
+      return $msg;
     }
   }
 
@@ -499,8 +511,9 @@ sub _validate_hashmap ($$;$) {
   # not contained in $consts_ref.
   foreach my $k (@map_keys) {
     if (scalar(grep {$_ eq $phase} @{$map_ref->{$k}->{'phases_opt'}}) == 0) {
-      $logger->error("$k is mandatory in phase $phase.");
-      return undef;
+      my $msg = "$k is mandatory in phase $phase.";
+      $logger->error($msg);
+      return $msg;
     }
   }
   return $consts_ref
@@ -523,37 +536,43 @@ sub _validate_hashmap ($$;$) {
 # \param phase the tool flow phase that defines the rules to check against
 #
 # \returns The given hash with all constants required in the design
-#          settings in sanitized form, or undef on errors.
+#          settings in sanitized form, or an error message.
 sub _sanitize_design {
   my $logger = get_logger();
   my ($consts_ref, $phase) = @_;
   if (ref($consts_ref) ne 'HASH') {
-    $logger->error("Parameter is not a reference to a hash (containing design constants).");
-    return undef;
+    my $msg = "Parameter is not a reference to a hash (containing design constants).";
+    $logger->error($msg);
+    return $msg;
   }
   
   # check for sane values
   $consts_ref = _validate_hashmap(DESIGNMAP, $consts_ref, $phase);
-  if (!defined($consts_ref)) {
-      $logger->error("Could not validate Design Constants.");
-      return undef;
+  if (!ref($consts_ref)) {
+      my $msg = "Could not validate Design Constants.";
+      $logger->error($msg);
+      return "$msg $consts_ref";
   }
 
   if (($consts_ref->{'FIU_CFG_BITS'} <= 0)) {
-      $logger->error("FIU_CFG_BITS is <= 0.");
-      return undef;
+      my $msg = "FIU_CFG_BITS is <= 0.";
+      $logger->error($msg);
+      return $msg;
   }
   if (($consts_ref->{'TIMER_WIDTH'} <= 0) || ($consts_ref->{'TIMER_WIDTH'} % 8 != 0)) {
-      $logger->error("TIMER_WIDTH is invalid ($consts_ref->{'TIMER_WIDTH'}).");
-      return undef;
+      my $msg = "TIMER_WIDTH is invalid ($consts_ref->{'TIMER_WIDTH'}).";
+      $logger->error($msg);
+      return $msg;
   }
   if (defined($consts_ref->{'ID'}) && ($consts_ref->{ID} < 0 || $consts_ref->{ID} > (2**15 - 1))) {
-      $logger->error("ID is invalid ($consts_ref->{ID}).");
-      return undef;
+      my $msg = "ID is invalid ($consts_ref->{ID}).";
+      $logger->error($msg);
+      return $msg;
   }
   if (($consts_ref->{'BAUDRATE'} <= 0)) {
-      $logger->error("BAUDRATE missing is <= 0.");
-      return undef;
+      my $msg = "BAUDRATE missing is <= 0.";
+      $logger->error($msg);
+      return $msg;
   }
 
   return $consts_ref;

fiji_download.pl

@@ -40,7 +40,10 @@ sub main {
   }
 
   my $fiji_settings = FIJI::Settings->new('download', $cfg{"cli.fiji_cfg"});
-  return 1 if !defined($fiji_settings);
+  if (!ref($fiji_settings)) {
+    $logger->error($fiji_settings);
+    return 1;
+  }
   my $fiji_consts = $fiji_settings->{'design'};
   # use Data::Dumper;
   # print Dumper(\%{cfg});

fiji_setup.pl

@@ -37,7 +37,10 @@ sub main {
   my $ini_name; # = $cfg{"cli.fiji_cfg"};
   $self->{'ini_name'} = \$ini_name;
   my $settings_ref = FIJI::Settings->new();
-  return 1 if !defined($settings_ref);
+  if (!ref($settings_ref)) {
+    $logger->error($settings_ref);
+    return 1;
+  }
   $self->{'settings'} = $settings_ref;
 
   my $mw = MainWindow->new(
@@ -118,12 +121,12 @@ sub _ctrl_frame {
         return;
       }
       my $tmp_settings = FIJI::Settings->new('setup', $filename, $self->{'settings'});
-      if (!defined($tmp_settings)) {
-        my $msg = "Settings file $filename could not be loaded correctly";
+      if (!ref($tmp_settings)) {
+        my $msg = "Settings file $filename could not be loaded correctly: $tmp_settings";
         $logger->error($msg);
         $fr->Dialog(
           -title => 'Open failed!',
-          -text => "$msg!\n",
+          -text => "$msg\n",
         )->Show();
         # ${$self->{'ini_name'}} = undef; leave old file name alone
         return;