Improve server polling performances (#255)

Do not sleep the process if about to send data to the device.  
Prevent sleeping uselessly between the reception of a response and the sending of the next one when doing active polling.

Author: pylessard

scrutiny/server/device/device_handler.py

@@ -35,6 +35,7 @@
 from scrutiny.server.protocol.comm_handler import CommHandler
 from scrutiny.server.protocol.commands import DummyCommand
 from scrutiny.server.device.request_dispatcher import RequestDispatcher, RequestRecord
+from scrutiny.server.device.submodules.base_device_handler_submodule import BaseDeviceHandlerSubmodule
 from scrutiny.server.device.submodules.device_searcher import DeviceSearcher
 from scrutiny.server.device.submodules.heartbeat_generator import HeartbeatGenerator
 from scrutiny.server.device.submodules.info_poller import InfoPoller
@@ -531,6 +532,26 @@ def get_link_type(self) -> str:
         """Returns what type of link is used to communicate with the device (serial, UDP, CanBus, etc)"""
         return self.comm_handler.get_link_type()
 
+    def need_keep_alive(self) -> bool:
+        """Inform the server layer that a call to process() will make meaningful work, therefore, avoid sleeping"""
+        if self.active_request_record is not None:  # waiting on data
+            return False
+
+        submodule: BaseDeviceHandlerSubmodule
+        for submodule in [
+            self.device_searcher,
+            self.session_initializer,
+            self.heartbeat_generator,
+            self.memory_reader,
+            self.memory_writer,
+            self.info_poller,
+            self.datalogging_poller,
+        ]:
+            if submodule.would_send_data():
+                return True
+
+        return False
+
     # Set communication state to a fresh start.
     def reset_comm(self) -> None:
         """Reset the communication with the device. Reset all state machines, clear pending requests, reset internal status"""
@@ -885,7 +906,7 @@ def do_state_machine(self) -> None:
                     )
 
             if self.disconnect_complete:
-                next_state != self.FsmState.DISCONNECTING
+                next_state = self.FsmState.INIT
 
         else:
             raise Exception('Unknown FSM state : %s' % self.fsm_state)

scrutiny/server/device/links/dummy_link.py

@@ -6,9 +6,7 @@
 #
 #   Copyright (c) 2022 Scrutiny Debugger
 
-__all__ = [
-    'DummyLink'
-]
+__all__ = ['DummyLink']
 
 from .abstract_link import AbstractLink, LinkConfig
 from scrutiny.tools.typing import *

scrutiny/server/device/links/rtt_link.py

@@ -6,10 +6,7 @@
 #
 #   Copyright (c) 2024 Scrutiny Debugger
 
-__all__ = [
-    'RttConfig',
-    'RttLink'
-]
+__all__ = ['RttLink']
 
 import logging
 import threading

scrutiny/server/device/links/serial_link.py

@@ -6,14 +6,12 @@
 #
 #   Copyright (c) 2022 Scrutiny Debugger
 
-__all__ = [
-    'SerialConfig',
-    'SerialLink'
-]
+__all__ = ['SerialLink']
 
 import logging
 import serial   # type: ignore
 import time
+import sys
 
 from scrutiny.server.device.links.abstract_link import AbstractLink, LinkConfig
 from scrutiny.server.device.links.typing import SerialConfigDict
@@ -76,7 +74,7 @@ def get_stop_bits(cls, s: Union[str, float, int]) -> float:
 
     @classmethod
     def get_data_bits(cls, s: Union[str, int]) -> int:
-        """ Converts a data bist input (str or number) to a pyserial constant"""
+        """ Converts a data bits input (str or number) to a pyserial constant"""
         try:
             s = int(s)
         except Exception:
@@ -168,7 +166,8 @@ def write(self, data: bytes) -> None:
             assert self.port is not None    # For mypy
             try:
                 self.port.write(data)
-                self.port.flush()
+                if sys.platform != 'win32':  # pyserial 3.5 :  Win32 implementation does nothing except sleeping
+                    self.port.flush()
             except Exception as e:
                 self.logger.debug(f"Cannot write data. {e}")
                 self.port.close()

scrutiny/server/device/links/udp_link.py

@@ -6,10 +6,7 @@
 #
 #   Copyright (c) 2022 Scrutiny Debugger
 
-__all__ = [
-    'UdpConfig',
-    'UdpLink'
-]
+__all__ = ['UdpLink']
 
 import logging
 import socket

scrutiny/server/device/submodules/base_device_handler_submodule.py

@@ -0,0 +1,25 @@
+
+import abc
+
+
+class BaseDeviceHandlerSubmodule(abc.ABC):
+    @abc.abstractmethod
+    def would_send_data(self) -> bool:
+        """Returns ``True`` if a call to ``process()`` would dispatch a request to the device"""
+        pass
+
+    @abc.abstractmethod
+    def start(self) -> None:
+        pass
+
+    @abc.abstractmethod
+    def stop(self) -> None:
+        pass
+
+    @abc.abstractmethod
+    def fully_stopped(self) -> bool:
+        pass
+
+    @abc.abstractmethod
+    def process(self) -> None:
+        pass

scrutiny/server/device/submodules/datalogging_poller.py

@@ -17,6 +17,7 @@
 from enum import Enum, auto
 from dataclasses import dataclass
 
+from scrutiny.server.device.submodules.base_device_handler_submodule import BaseDeviceHandlerSubmodule
 from scrutiny.server.protocol import *
 from scrutiny.server.device.request_dispatcher import RequestDispatcher
 import scrutiny.server.datalogging.definitions.device as device_datalogging
@@ -69,7 +70,7 @@ class _ReceivedChunk:
 DatalogSubfn = cmd.DatalogControl.Subfunction
 
 
-class DataloggingPoller:
+class DataloggingPoller(BaseDeviceHandlerSubmodule):
 
     UPDATE_STATUS_INTERVAL_IDLE = 0.5
     UPDATE_STATUS_INTERVAL_ACQUIRING = 0.2
@@ -327,6 +328,19 @@ def is_ready_to_receive_new_request(self) -> bool:
         """Tells if request_acquisition() can be called. """
         return self.started and not self.error and not self.cancel_requested and not self.stop_requested and self.device_setup is not None
 
+    def would_send_data(self) -> bool:
+        # We don't need to be accurate here.
+        # It's only an optimization to prevent the server from sleeping.
+        # Since we sent occasional message, optimizing few milliseconds is not worth it.
+
+        if not self.started or not self.enabled or self.stop_requested or self.error:
+            return False
+
+        if self.require_status_update or self.update_status_timer.is_timed_out():
+            return True
+        # Don't bother about all the other possibilities, not worth it.
+        return False
+
     def process(self) -> None:
         """To be called periodically to make the process move forward"""
         # Handle conditions that prevent the DataloggingPoller to function
@@ -504,7 +518,6 @@ def process(self) -> None:
 
                 if self.cancel_requested:   # New request interrupts the previous one
                     if not self.request_pending[DatalogSubfn.ReadAcquisition]:
-                        self.must_send_read_data_request = False
                         next_state = _FSMState.REQUEST_RESET
 
                 elif self.device_datalogging_state == device_datalogging.DataloggerState.ERROR:

scrutiny/server/device/submodules/device_searcher.py

@@ -14,14 +14,15 @@
 import binascii
 import traceback
 
+from scrutiny.server.device.submodules.base_device_handler_submodule import BaseDeviceHandlerSubmodule
 from scrutiny.server.protocol import *
 import scrutiny.server.protocol.typing as protocol_typing
 from scrutiny.server.device.request_dispatcher import RequestDispatcher
 
 from scrutiny.tools.typing import *
 
 
-class DeviceSearcher:
+class DeviceSearcher(BaseDeviceHandlerSubmodule):
     """
     Generates Discover request in loop and inform the upper layers if a device has been found
     """
@@ -105,6 +106,14 @@ def get_device_protocol_version(self) -> Optional[Tuple[int, int]]:
             return (self.found_device['protocol_major'], self.found_device['protocol_minor'])
         return None
 
+    def _search_request_timedout(self) -> bool:
+        return self.last_request_timestamp is None or (time.monotonic() - self.last_request_timestamp > self.DISCOVER_INTERVAL)
+
+    def would_send_data(self) -> bool:
+        if not self.started or self.pending:
+            return False
+        return self._search_request_timedout()
+
     def process(self) -> None:
         """To be called periodically"""
         if not self.started:
@@ -116,7 +125,7 @@ def process(self) -> None:
             self.found_device = None
 
         if self.pending == False:
-            if self.last_request_timestamp is None or (time.monotonic() - self.last_request_timestamp > self.DISCOVER_INTERVAL):
+            if self._search_request_timedout():
                 if self.logger.isEnabledFor(logging.DEBUG):  # pragma: no cover
                     self.logger.debug('Registering a Discover request')
                 self.dispatcher.register_request(

scrutiny/server/device/submodules/heartbeat_generator.py

@@ -12,14 +12,15 @@
 import time
 import logging
 
+from scrutiny.server.device.submodules.base_device_handler_submodule import BaseDeviceHandlerSubmodule
 from scrutiny.server.protocol import *
 import scrutiny.server.protocol.typing as protocol_typing
 from scrutiny.server.device.request_dispatcher import RequestDispatcher
 from scrutiny import tools
 from scrutiny.tools.typing import *
 
 
-class HeartbeatGenerator:
+class HeartbeatGenerator(BaseDeviceHandlerSubmodule):
     """
     Poll the device with periodic heartbeat message to know if it is still there and alive.
     """
@@ -32,15 +33,15 @@ class HeartbeatGenerator:
     """Our dispatcher priority"""
     session_id: Optional[int]
     """The session ID to include in the heartbeat request"""
-    last_heartbeat_request: Optional[float]
+    last_heartbeat_request_timestamp: Optional[float]
     """Time at which that last heartbeat request has been sent."""
     last_heartbeat_timestamp: Optional[float]
     """Time at which the last successful heartbeat response has been received"""
     challenge: int
-    """The computation challenge included in the pending request."""
+    """The computation challenge included in the request_pending request."""
     interval: float
     """Heartbeat interval in seconds"""
-    pending: bool
+    request_pending: bool
     """True when a request is sent and we are waiting for a response"""
     started: bool
     """True when started. Sends heartbeat only when started, otherwise keep silent"""
@@ -50,7 +51,7 @@ def __init__(self, protocol: Protocol, dispatcher: RequestDispatcher, priority:
         self.dispatcher = dispatcher
         self.protocol = protocol
         self.session_id = None
-        self.last_heartbeat_request = None
+        self.last_heartbeat_request_timestamp = None
         self.last_heartbeat_timestamp = time.monotonic()
         self.challenge = 0
         self.interval = 3
@@ -76,24 +77,36 @@ def stop(self) -> None:
         self.started = False
 
     def fully_stopped(self) -> bool:
-        """Indicates that this submodule is stopped and has no pending state"""
+        """Indicates that this submodule is stopped and has no request_pending state"""
         return self.started == False
 
     def reset(self) -> None:
         """Put the heartbeat generator in its startup state"""
-        self.pending = False
+        self.request_pending = False
         self.started = False
         self.session_id = None
 
+    def _heartbeat_timedout(self) -> bool:
+        return self.last_heartbeat_request_timestamp is None or (time.monotonic() - self.last_heartbeat_request_timestamp > self.interval)
+
+    def would_send_data(self) -> bool:
+        if not self.started:
+            return False
+
+        if self.request_pending or self.session_id is None:
+            return False
+
+        return self._heartbeat_timedout()
+
     def process(self) -> None:
         """To be called periodically"""
         if not self.started:
             self.reset()
             return
 
         # If no request is being waited and we have a session ID assigned
-        if self.pending == False and self.session_id is not None:
-            if self.last_heartbeat_request is None or (time.monotonic() - self.last_heartbeat_request > self.interval):
+        if self.request_pending == False and self.session_id is not None:
+            if self._heartbeat_timedout():
                 if self.logger.isEnabledFor(logging.DEBUG):  # pragma: no cover
                     self.logger.debug('Registering a Heartbeat request')
                 self.dispatcher.register_request(
@@ -102,8 +115,8 @@ def process(self) -> None:
                     failure_callback=self._failure_callback,
                     priority=self.priority
                 )
-                self.pending = True
-                self.last_heartbeat_request = time.monotonic()
+                self.request_pending = True
+                self.last_heartbeat_request_timestamp = time.monotonic()
 
     def _success_callback(self, request: Request, response: Response, params: Any = None) -> None:
         """ Called by the dispatcher when a request is completed and succeeded"""
@@ -140,4 +153,4 @@ def _failure_callback(self, request: Request, params: Any = None) -> None:
     def _completed(self) -> None:
         """ Common code between success and failure"""
         self.challenge = (self.challenge + 1) & 0xFFFF  # Next challenge
-        self.pending = False
+        self.request_pending = False

scrutiny/server/device/submodules/info_poller.py

@@ -18,6 +18,7 @@
 import enum
 import copy
 
+from scrutiny.server.device.submodules.base_device_handler_submodule import BaseDeviceHandlerSubmodule
 from scrutiny.core.basic_types import MemoryRegion
 from scrutiny.server.protocol import ResponseCode
 from scrutiny.server.device.device_info import *
@@ -35,7 +36,7 @@
 CommParamCallback = Callable[[DeviceInfo], Any]
 
 
-class InfoPoller:
+class InfoPoller(BaseDeviceHandlerSubmodule):
     """Class that will successfully sends polling request to the device
     to gather all of its internal parameters. Will fill a DeviceInformation structure"""
 
@@ -155,6 +156,17 @@ def reset(self) -> None:
         self.error_message = ""
         self.info.clear()
 
+    def would_send_data(self) -> bool:
+        if not self.started:
+            return False
+        if self.stop_requested:
+            return False
+        if self.request_pending:
+            return False
+        if self.fsm_state in (self.FsmState.Done, self.FsmState.Error):
+            return False
+        return True
+
     def process(self) -> None:
         """To be called  periodically to make the process move forward"""
         if not self.started: