Fix generation not being incremented in discovery node

misc/discoverynode/src/tests/test_discovery.py

@@ -13,6 +13,8 @@
 import udmi.schema.util 
 import logging
 import sys
+import collections
+import math
 
 stdout = logging.StreamHandler(sys.stdout)
 stdout.addFilter(lambda log: log.levelno < logging.WARNING)
@@ -159,4 +161,51 @@ def test_generation_scheduling(seconds_from_now, scan_interval, threshold, expec
 
     # hacky because timestamps are generted from datetime and timestamp
     # needs to mock all the objects to make it work
-    assert pytest.approx(expected_delay + 1000, abs=1) == start_time
+    assert pytest.approx(expected_delay + 1000, abs=1) == start_time
+
+@pytest.mark.parametrize(
+  "scan_duration, scan_interval",
+  [
+    # Set to more than one second to avoid issues with state message publishing
+    (None, 1),
+    (None, 3),
+    (1, 3)
+
+  ]
+)
+def test_generation_is_incremented(scan_duration, scan_interval):
+  mock_state = state.State()
+  mock_publisher = mock.MagicMock()
+  numbers =  udmi.discovery.numbers.NumberDiscovery(mock_state, mock_publisher)
+
+  cycles = 5
+  got = set()
+
+  initial_generation = udmi.schema.util.current_time_utc()
+
+  with (
+    mock.patch.object(numbers, "start_discovery") as mock_start,
+  ):
+
+    numbers.controller({"discovery": {
+      "families": {
+        "vendor" : {
+          "generation": udmi.schema.util.datetime_serializer(initial_generation), 
+          "scan_interval_sec": scan_interval, 
+          "scan_duration_sec": scan_duration
+          }
+        }
+      }
+    })
+
+    for x in range(300):
+      got.add(udmi.schema.util.datetime_serializer(mock_state.discovery.families["vendor"].generation))
+      if len(got) == cycles:
+        break
+      time.sleep(0.1)
+
+    want = set([udmi.schema.util.datetime_serializer(initial_generation +  datetime.timedelta(seconds=scan_interval * n)) for n in range(cycles)])
+
+    assert want == got
+    # Below disabled because flakey
+    #assert mock_start.call_count == cycles

misc/discoverynode/src/udmi/discovery/discovery.py

@@ -272,10 +272,11 @@ def _scheduler(self, start_time:int, config: udmi.schema.config.DiscoveryFamily,
 
             if scan_duration_sec > 0:
               next_action = ACTION_STOP
-              # Calcultae based of generation and current time, in case the generation is in the past
+              # Calculate based of generation and current time, in case the generation is in the past
               # Otherwise systematic error gets introduced into all repetitive measurements
               # e.g. instead of starting "on the hour", they all start 5 seconds pst the hour
-              next_action_time = time.time() + scan_duration_sec
+              scheduled_stop_time = self.generation + datetime.timedelta(seconds=scan_duration_sec)
+              next_action_time = scheduled_stop_time.timestamp()
               logging.info("scheduled discovery stop for %s in %d seconds", type(self).__name__, scan_duration_sec)
             else:
               # the scan runs indefinitely, exit the scheduler
@@ -288,9 +289,9 @@ def _scheduler(self, start_time:int, config: udmi.schema.config.DiscoveryFamily,
             # If the scan is repetitive, schedule the next start
             if scan_interval_sec > 0:
               next_action = ACTION_START
-              sleep_interval = scan_interval_sec - scan_duration_sec
-              # calculate the next generation
-              next_action_time = time.time() + sleep_interval
+              next_generation = self.generation + datetime.timedelta(seconds=scan_interval_sec)
+              next_action_time = next_generation.timestamp()
+              self.set_generation(next_generation)
               logging.info("scheduled discovery start for %s in %d seconds", type(self).__name__, scan_duration_sec)
               self._set_internal_state(states.SCHEDULED)
               self.state.phase = udmi.schema.state.Phase.pending
@@ -415,8 +416,8 @@ def set_generation(self, new_generation:datetime.datetime) -> None:
     """Updates the generation to the the given generation"""
     current_generation = udmi.schema.util.datetime_serializer(self.generation) if self.generation else None
     logging.info("generation now %s, was %s", 
-                 current_generation,
-                 udmi.schema.util.datetime_serializer(new_generation))
+                 udmi.schema.util.datetime_serializer(new_generation),
+                 current_generation)
     self.generation = new_generation
     self.state.generation = new_generation
 

misc/discoverynode/src/udmi/discovery/numbers.py

@@ -51,4 +51,5 @@ def discoverer(self):
 
   def stop_discovery(self):
     self.cancelled = True
-    self.task_thread.join()
+    if self.task_thread:
+      self.task_thread.join()