Changeset 14767

Timestamp:

01/09/13 16:11:42 (12 years ago)

Author:

neise

Message:

added some comments containing questions, ideas and remarks

File:

• trunk/FACT++/scripts/Main.js (modified) (8 diffs)

trunk/FACT++/scripts/Main.js

@@ -76 +76 @@
 // ----------------------------------------------------------------
 
-
+//DN: the name of the Subscription object 'service_con' is not really 
+// telling that its a subscription to FAD_CONTROL/CONNECTIONS
+// fad_connections sound ok for be, since 
+// fad_connections.onchange() is pretty clear
+// fad_connections.reconnect() is not really good, but at least it has FAD in it.
 var service_con = new Subscription("FAD_CONTROL/CONNECTIONS");
+/**
+ * call-back function of FAD_CONTROL/CONNECTIONS
+ * store IDs of problematic FADs 
+ *
+ */
 service_con.onchange = function(evt)
 {
@@ -94 +103 @@
 }
 
+/**
+ * reconnect to problematic FADs
+ *
+ * Dis- and Reconnects to FADs, found to be problematic by call-back function
+ * onchange() to have a different CONNECTION value than 66 or 67. 
+ * 
+ * @returns
+ *      a boolean is returned. 
+ *      reconnect returns true if:
+ *          * nothing needed to be reset --> no problems found by onchange()
+ *          * the reconnection went fine.
+ *      
+ *      reconnect *never returns false* so far.
+ *
+ * @example
+ *      if (!service_con.reconnect())
+ *          exit();
+ */
 service_con.reconnect = function()
 {
+    // this.reset is a list containing the IDs of FADs, 
+    // which have neither CONNECTION==66 nor ==67, whatever this means :-)
     if (this.reset.length==0)
         return true;
@@ -129 +158 @@
     dim.send("MCP/START", time?time:-1, count?count:-1, type);
 
+    
+
     // What could be a reasonable timeout here?
     // FIXME: Replace by callback?
+    //
+    // DN: I believe instead of waiting for 'TakingData' one could split this
+    // up into two checks with an extra condition:
+    //  if type == 'data':
+    //      wait until ThresholdCalibration starts:
+    //          --> this time should be pretty identical for each run
+    //      if this takes longer than say 3s:
+    //          there might be a problem with one/more FADs
+    //    
+    //      wait until "TakingData":
+    //          --> this seems to take even some minutes sometimes... 
+    //              (might be optimized rather soon, but still in the moment...)
+    //      if this takes way too long: 
+    //          there might be something broken, 
+    //          so maybe a very high time limit is ok here.
+    //          I think there is not much that can go wrong, 
+    //          when the Thr-Calib has already started. Still it might be nice 
+    //          If in the future RateControl is written so to find out that 
+    //          in case the threshold finding algorithm does 
+    //          *not converge as usual*
+    //          it can complain, and in this way give a hint, that the weather
+    //          might be a little bit too bad.
+    //  else:
+    //      wait until "TakingData":
+    //          --> in a non-data run this time should be pretty short again
+    //      if this takes longer than say 3s:
+    //          there might be a problem with one/more FADs
+    //  
+
     if (!dim.wait("MCP", "TakingData", -300000) )
     {
@@ -137 +197 @@
         dimctrl.setState(37);
         dim.wait("MCP", "TakingData", 500);
-
     }
     dim.wait("MCP", "Idle");
@@ -143 +202 @@
     console.out("  Take run: end");
 
+    // DN: currently reconnect() never returns false 
+    //     .. but it can fail of course.
     if (!service_con.reconnect())
         exit();
@@ -224 +285 @@
 var service_feedback = new Subscription("FEEDBACK/DEVIATION");
 
+// DN:  Why is voltageOff() implemented as 
+//      a method of a Subscription to a specific Service
+//      I naively would think of voltageOff() as an unbound function.
+//      I seems to me it has to be a method of a Subscription object, in order
+//      to use the update counting method. But does it have to be
+//      a Subscription to FEEDBACK/DEVIATION, or could it work with other services as well?
+service_feedback.voltageOff = function()
+{
+    console.out("  Voltage off: start");
+
+    var isOn = dim.state("BIAS_CONTROL").name=="VoltageOn";
+
+    if (isOn)
+    {
+        console.out("  Voltage on: switch off");
+        dim.send("BIAS_CONTROL/SET_ZERO_VOLTAGE");
+    }
+
+    dim.wait("BIAS_CONTROL", "VoltageOff", 5000);
+
+    // FEEDBACK stays in CurrentCtrl when Voltage is off but output enabled
+    // dim.wait("FEEDBACK", "CurrentCtrlIdle", 1000);
+
+    console.out("  Voltage off: end");
+}
+
+// DN:  The name of the method voltageOn() in the context of the method
+//      voltageOff() is a little bit misleading, since when voltageOff() returns
+//      the caller can be sure the voltage is off, but when voltageOn() return
+//      this is not the case, in the sense, that the caller can now take data.
+//      instead the caller of voltageOn() *must* call waitForVoltageOn() afterwards
+//      in order to safely take good-quality data.
+//      This could lead to nasty bugs in the sense, that the second call might 
+//      be forgotten by somebody
+//      
+//      so I suggest to rename voltageOn() --> prepareVoltageOn()
+//      waitForVoltageOn() stays as it is
+//      and one creates a third method called:voltageOn() like this
+/*      service_feedback.voltageOn = function()
+ *      {
+ *          this.prepareVoltageOn();
+ *          this.waitForVoltageOn();
+ *      }
+ * 
+ * */
+//      For convenience.
+
 service_feedback.voltageOn = function()
 {
@@ -257 +365 @@
 }
 
-service_feedback.voltageOff = function()
-{
-    console.out("  Voltage off: start");
-
-    var isOn = dim.state("BIAS_CONTROL").name=="VoltageOn";
-
-    if (isOn)
-    {
-        console.out("  Voltage on: switch off");
-        dim.send("BIAS_CONTROL/SET_ZERO_VOLTAGE");
-    }
-
-    dim.wait("BIAS_CONTROL", "VoltageOff", 5000);
-
-    // FEEDBACK stays in CurrentCtrl when Voltage is off but output enabled
-    // dim.wait("FEEDBACK", "CurrentCtrlIdle", 1000);
-
-    console.out("  Voltage off: end");
-}
 
 
@@ -436 +525 @@
     return observations.length-1;
 }
+
+
+// DN: using so called magic numbers to encode certain 
+//      states of the logic is considered pretty bad coding as far as I understand.
+//      the reader at this point has no idea why run is -2 ... this is the first time she
+//      reads about this variable and there is not a word of explanation found.
 var run = -2;
 var lastObs;