shithub: m8c

Download patch

ref: 62586fcbd20e37d362b620e40de29363af5166ad
parent: b892a5ab495e3d92a14955d8daf9fece5c651167
author: Jonne Kokkonen <jonne.kokkonen@gmail.com>
date: Mon Mar 24 18:29:53 EDT 2025

Refactor and improve M8 initialization and input handling

Streamlined device initialization by modularizing functions and improving error handling with clear return codes. Refactored the main loop for better readability.

--- a/src/backends/m8.h
+++ b/src/backends/m8.h
@@ -5,13 +5,19 @@
 
 #include "../config.h"
 
+enum return_codes {
+  DEVICE_DISCONNECTED = 0,
+  DEVICE_PROCESSING = 1,
+  DEVICE_FATAL_ERROR = -1
+};
+
 int m8_initialize(int verbose, const char *preferred_device);
 int m8_list_devices(void);
 int m8_reset_display(void);
 int m8_enable_and_reset_display(void);
-int m8_send_msg_controller(const unsigned char input);
-int m8_send_msg_keyjazz(const unsigned char note, unsigned char velocity);
-int m8_process_data(config_params_s conf);
+int m8_send_msg_controller(unsigned char input);
+int m8_send_msg_keyjazz(unsigned char note, unsigned char velocity);
+int m8_process_data(const config_params_s *conf);
 int m8_close(void);
 
 #endif
\ No newline at end of file
--- a/src/backends/m8_libserialport.c
+++ b/src/backends/m8_libserialport.c
@@ -13,8 +13,8 @@
 #include "../command.h"
 #include "../config.h"
 #include "m8.h"
-#include "slip.h"
 #include "queue.h"
+#include "slip.h"
 
 #define SERIAL_READ_SIZE 1024  // maximum amount of bytes to read from the serial in one pass
 #define SERIAL_READ_DELAY_MS 4 // delay between serial reads in milliseconds
@@ -24,7 +24,6 @@
 static uint8_t serial_buffer[SERIAL_READ_SIZE] = {0};
 static uint8_t slip_buffer[SERIAL_READ_SIZE] = {0};
 static slip_handler_s slip;
-static uint16_t zero_byte_packets = 0; // used to detect device disconnection
 message_queue_s queue;
 
 SDL_Thread *serial_thread = NULL;
@@ -39,44 +38,20 @@
 // Helper function for error handling
 static int check(enum sp_return result);
 
-int send_message_to_queue(uint8_t *data, const uint32_t size) {
+static int send_message_to_queue(uint8_t *data, const uint32_t size) {
   push_message(&queue, data, size);
   return 1;
 }
 
-int m8_send_msg_controller(const uint8_t input) {
-  const char buf[2] = {'C', input};
-  const size_t nbytes = 2;
-  const int result = sp_blocking_write(m8_port, buf, nbytes, 5);
-  if (result != nbytes) {
-    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error sending input, code %d", result);
-    return -1;
-  }
-  return 1;
-}
-
-int m8_send_msg_keyjazz(const uint8_t note, uint8_t velocity) {
-  if (velocity > 0x7F)
-    velocity = 0x7F;
-  const char buf[3] = {'K', note, velocity};
-  const size_t nbytes = 3;
-  const int result = sp_blocking_write(m8_port, buf, nbytes, 5);
-  if (result != nbytes) {
-    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error sending keyjazz, code %d", result);
-    return -1;
-  }
-
-  return 1;
-}
-
-int disconnect() {
+static int disconnect() {
   SDL_Log("Disconnecting M8");
 
   // wait for serial processing thread to finish
   thread_params.should_stop = 1;
   SDL_WaitThread(serial_thread, NULL);
+  destroy_queue(&queue);
 
-  const char buf[1] = {'D'};
+  const unsigned char buf[1] = {'D'};
 
   int result = sp_blocking_write(m8_port, buf, 1, 5);
   if (result != 1) {
@@ -83,6 +58,7 @@
     SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error sending disconnect, code %d", result);
     result = 0;
   }
+
   sp_close(m8_port);
   sp_free_port(m8_port);
   m8_port = NULL;
@@ -105,27 +81,6 @@
   return 0;
 }
 
-int m8_list_devices() {
-  struct sp_port **port_list;
-  const enum sp_return result = sp_list_ports(&port_list);
-
-  if (result != SP_OK) {
-    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "sp_list_ports() failed!\n");
-    return 1;
-  }
-
-  for (int i = 0; port_list[i] != NULL; i++) {
-    const struct sp_port *port = port_list[i];
-
-    if (detect_m8_serial_device(port)) {
-      SDL_Log("Found M8 device: %s", sp_get_port_name(port));
-    }
-  }
-
-  sp_free_port_list(port_list);
-  return 0;
-}
-
 static void process_received_bytes(const uint8_t *buffer, int bytes_read, slip_handler_s *slip) {
   const uint8_t *cur = buffer;
   const uint8_t *end = buffer + bytes_read;
@@ -137,8 +92,8 @@
   }
 }
 
-int thread_process_serial_data(void *data) {
-  thread_params_s *thread_params = data;
+static int thread_process_serial_data(void *data) {
+  const thread_params_s *thread_params = data;
 
   while (!thread_params->should_stop) {
     // attempt to read from serial port
@@ -175,38 +130,67 @@
   return 1;
 }
 
-int m8_initialize(const int verbose, const char *preferred_device) {
-  if (m8_port != NULL) {
-    // Port is already initialized
-    return 1;
+// Helper function for error handling.
+static int check(const enum sp_return result) {
+  char *error_message;
+
+  switch (result) {
+  case SP_ERR_ARG:
+    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Invalid argument");
+    break;
+  case SP_ERR_FAIL:
+    error_message = sp_last_error_message();
+    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Failed: %s", error_message);
+    sp_free_error_message(error_message);
+    break;
+  case SP_ERR_SUPP:
+    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Not supported");
+    break;
+  case SP_ERR_MEM:
+    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Couldn't allocate memory");
+    break;
+  case SP_OK:
+  default:
+    break;
   }
+  return result;
+}
 
-  // settings for the slip packet handler
-  static const slip_descriptor_s slip_descriptor = {
-      .buf = slip_buffer,
-      .buf_size = sizeof(slip_buffer),
-      .recv_message = send_message_to_queue, // complete slip packets callback
-  };
+// Extracted function for initializing threads and message queue
+static int initialize_serial_thread() {
 
-  slip_init(&slip, &slip_descriptor);
+  init_queue(&queue);
+  thread_params.should_stop = 0;
+  serial_thread = SDL_CreateThread(thread_process_serial_data, "SerialThread", &thread_params);
 
-  if (verbose)
-    SDL_Log("Looking for USB serial devices.\n");
+  if (!serial_thread) {
+    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "SDL_CreateThread Error: %s", SDL_GetError());
+    SDL_Quit();
+    return 0;
+  }
+
+  return 1;
+}
+
+// Extracted function for detecting and selecting the M8 device
+static int find_and_select_device(const char *preferred_device) {
   struct sp_port **port_list;
-  enum sp_return port_result = sp_list_ports(&port_list);
+  const enum sp_return port_result = sp_list_ports(&port_list);
+
   if (port_result != SP_OK) {
-    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "sp_list_ports() failed!\n");
+    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "sp_list_ports() failed!");
     return 0;
   }
 
-  // Iterate through the ports. When port_list[i] is NULL this indicates the end of the list.
   for (int i = 0; port_list[i] != NULL; i++) {
     const struct sp_port *port = port_list[i];
 
     if (detect_m8_serial_device(port)) {
       char *port_name = sp_get_port_name(port);
-      SDL_Log("Found M8 in %s.\n", port_name);
+      SDL_Log("Found M8 in %s", port_name);
       sp_copy_port(port, &m8_port);
+
+      // Break if preferred device is found
       if (preferred_device != NULL && strcmp(preferred_device, port_name) == 0) {
         SDL_Log("Found preferred device, breaking");
         break;
@@ -215,60 +199,94 @@
   }
 
   sp_free_port_list(port_list);
+  return (m8_port != NULL);
+}
 
-  if (m8_port == NULL) {
-    if (verbose)
+int m8_initialize(const int verbose, const char *preferred_device) {
+  if (m8_port != NULL) {
+    // Port is already initialized
+    return 1;
+  }
+
+  // Initialize slip descriptor
+  static const slip_descriptor_s slip_descriptor = {
+      .buf = slip_buffer,
+      .buf_size = sizeof(slip_buffer),
+      .recv_message = send_message_to_queue,
+  };
+  slip_init(&slip, &slip_descriptor);
+
+  if (verbose) {
+    SDL_Log("Looking for USB serial devices.\n");
+  }
+
+  // Detect and select M8 device
+  if (!find_and_select_device(preferred_device)) {
+    if (verbose) {
       SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Cannot find a M8");
+    }
     return 0;
   }
 
-  SDL_Log("Opening and configuring port");
+  // Configure serial port
   if (!configure_serial_port(m8_port)) {
     return 0;
   }
 
-  init_queue(&queue);
-  thread_params.should_stop = 0;
-  serial_thread = SDL_CreateThread(thread_process_serial_data, "SerialThread", &thread_params);
+  // Initialize message queue and threads
+  return initialize_serial_thread();
+}
 
-  if (!serial_thread) {
-    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "SDL_CreateThread Error: %s\n", SDL_GetError());
-    SDL_Quit();
-    return 0;
+int m8_send_msg_controller(const uint8_t input) {
+  const unsigned char buf[2] = {'C', input};
+  const size_t nbytes = 2;
+  const int result = sp_blocking_write(m8_port, buf, nbytes, 5);
+  if (result != nbytes) {
+    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error sending input, code %d", result);
+    return -1;
   }
   return 1;
 }
 
-// Helper function for error handling.
-static int check(const enum sp_return result) {
-  char *error_message;
+int m8_send_msg_keyjazz(const uint8_t note, uint8_t velocity) {
+  if (velocity > 0x7F)
+    velocity = 0x7F;
+  const unsigned char buf[3] = {'K', note, velocity};
+  const size_t nbytes = 3;
+  const int result = sp_blocking_write(m8_port, buf, nbytes, 5);
+  if (result != nbytes) {
+    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error sending keyjazz, code %d", result);
+    return -1;
+  }
 
-  switch (result) {
-  case SP_ERR_ARG:
-    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Invalid argument.\n");
-    break;
-  case SP_ERR_FAIL:
-    error_message = sp_last_error_message();
-    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Failed: %s\n", error_message);
-    sp_free_error_message(error_message);
-    break;
-  case SP_ERR_SUPP:
-    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Not supported.\n");
-    break;
-  case SP_ERR_MEM:
-    SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Couldn't allocate memory.\n");
-    break;
-  case SP_OK:
-  default:
-    break;
+  return 1;
+}
+
+int m8_list_devices() {
+  struct sp_port **port_list;
+  const enum sp_return result = sp_list_ports(&port_list);
+
+  if (result != SP_OK) {
+    SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "sp_list_ports() failed!\n");
+    return 1;
   }
-  return result;
+
+  for (int i = 0; port_list[i] != NULL; i++) {
+    const struct sp_port *port = port_list[i];
+
+    if (detect_m8_serial_device(port)) {
+      SDL_Log("Found M8 device: %s", sp_get_port_name(port));
+    }
+  }
+
+  sp_free_port_list(port_list);
+  return 0;
 }
 
 int m8_reset_display() {
   SDL_Log("Reset display\n");
 
-  const char buf[1] = {'R'};
+  const unsigned char buf[1] = {'R'};
   const int result = sp_blocking_write(m8_port, buf, 1, 5);
   if (result != 1) {
     SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error resetting M8 display, code %d", result);
@@ -292,12 +310,12 @@
   return result;
 }
 
-int m8_process_data(const config_params_s conf) {
+int m8_process_data(const config_params_s *conf) {
   static unsigned int empty_cycles = 0;
 
   // Device likely has been disconnected
   if (m8_port == NULL) {
-    return 0;
+    return DEVICE_DISCONNECTED;
   }
 
   if (queue_size(&queue) > 0) {
@@ -305,18 +323,23 @@
     empty_cycles = 0;
     size_t length = 0;
     while ((command = pop_message(&queue, &length)) != NULL) {
-      process_command(command, length);
+      if (length > 0) {
+        process_command(command, length);
+      }
       SDL_free(command);
     }
   } else {
     empty_cycles++;
-    if (empty_cycles >= conf.wait_packets) {
-      SDL_Log("No messages received for %d cycles, assuming device disconnected", empty_cycles);
+    if (empty_cycles >= conf->wait_packets) {
+      SDL_LogError(SDL_LOG_CATEGORY_SYSTEM,
+                   "No messages received for %d cycles, assuming device disconnected",
+                   empty_cycles);
+      empty_cycles = 0;
       disconnect();
-      return 0;
+      return DEVICE_DISCONNECTED;
     }
   }
-  return 1;
+  return DEVICE_PROCESSING;
 }
 
 int m8_close() { return disconnect(); }
--- a/src/backends/queue.h
+++ b/src/backends/queue.h
@@ -26,6 +26,13 @@
 void init_queue(message_queue_s *queue);
 
 /**
+ * Destroys the message queue and releases associated resources.
+ *
+ * @param queue A pointer to the message queue structure to be destroyed.
+ */
+void destroy_queue(message_queue_s *queue);
+
+/**
  * Retrieves and removes a message from the front of the message queue.
  * If the queue is empty, the function returns NULL.
  *
--- a/src/input.c
+++ b/src/input.c
@@ -14,12 +14,12 @@
 
 static input_msg_s key = {normal, 0, 0, 0};
 
-int input_process(config_params_s conf, enum app_state *app_state) {
+int input_process(config_params_s *conf, enum app_state *app_state) {
   static uint8_t prev_input = 0;
   static uint8_t prev_note = 0;
 
   // get current inputs
-  const input_msg_s input = input_get_msg(&conf);
+  const input_msg_s input = input_get_msg(conf);
 
   switch (input.type) {
   case normal:
@@ -51,8 +51,8 @@
         m8_reset_display();
         break;
       case msg_toggle_audio:
-        conf.audio_enabled = !conf.audio_enabled;
-        audio_toggle(conf.audio_device_name, conf.audio_buffer_size);
+        conf->audio_enabled = !conf->audio_enabled;
+        audio_toggle(conf->audio_device_name, conf->audio_buffer_size);
         break;
       default:
         break;
--- a/src/input.h
+++ b/src/input.h
@@ -49,6 +49,6 @@
 } input_msg_s;
 
 input_msg_s input_get_msg(config_params_s *conf);
-int input_process(config_params_s conf, enum app_state *app_state);
+int input_process(config_params_s *conf, enum app_state *app_state);
 
 #endif
--- a/src/main.c
+++ b/src/main.c
@@ -17,15 +17,26 @@
 #include "input.h"
 #include "render.h"
 
+#include <stdlib.h>
+
 enum app_state app_state = WAIT_FOR_DEVICE;
 
 // Handle CTRL+C / SIGINT, SIGKILL etc.
-void signal_handler(int unused) {
+static void signal_handler(int unused) {
   (void)unused;
   app_state = QUIT;
 }
 
-void do_wait_for_device(const char *preferred_device, unsigned char *m8_connected,
+static void initialize_signals() {
+  signal(SIGINT, signal_handler);
+  signal(SIGTERM, signal_handler);
+#ifdef SIGQUIT
+  signal(SIGQUIT, signal_handler);
+#endif
+}
+
+
+static void do_wait_for_device(const char *preferred_device, unsigned char *m8_connected,
                         config_params_s *conf) {
   static Uint64 ticks_poll_device = 0;
   static Uint64 ticks_update_screen = 0;
@@ -77,124 +88,114 @@
   }
 }
 
-int main(const int argc, char *argv[]) {
-
-  char *preferred_device = NULL;
-  unsigned char m8_connected = 0;
-
-  if (argc == 2 && SDL_strcmp(argv[1], "--list") == 0) {
-    return m8_list_devices();
+static config_params_s initialize_config(int argc, char *argv[], char **preferred_device, char **config_filename) {
+  for (int i = 1; i < argc; i++) {
+    if (SDL_strcmp(argv[i], "--list") == 0) {
+      exit(m8_list_devices());
+    }
+    if (SDL_strcmp(argv[i], "--dev") == 0 && i + 1 < argc) {
+      *preferred_device = argv[i + 1];
+      SDL_Log("Using preferred device: %s.\n", *preferred_device);
+      i++;
+    } else if (SDL_strcmp(argv[i], "--config") == 0 && i + 1 < argc) {
+      *config_filename = argv[i + 1];
+      SDL_Log("Using config file: %s.\n", *config_filename);
+      i++;
+    }
   }
 
-  if (argc == 3 && SDL_strcmp(argv[1], "--dev") == 0) {
-    preferred_device = argv[2];
-    SDL_Log("Using preferred device %s.\n", preferred_device);
-  }
-
-  char *config_filename = NULL;
-  if (argc == 3 && SDL_strcmp(argv[1], "--config") == 0) {
-    config_filename = argv[2];
-    SDL_Log("Using config file %s.\n", config_filename);
-  }
-
-  // Initialize the config to defaults
-  config_params_s conf = config_initialize(config_filename);
-  // Read in the params from the configfile if present
+  config_params_s conf = config_initialize(*config_filename);
   config_read(&conf);
+  return conf;
+}
 
-  signal(SIGINT, signal_handler);
-  signal(SIGTERM, signal_handler);
-#ifdef SIGQUIT
-  signal(SIGQUIT, signal_handler);
-#endif
-
-  // First device detection to avoid SDL init if it isn't necessary. To be run
-  // only if we shouldn't wait for M8 to be connected.
-  if (conf.wait_for_device == 0) {
-    m8_connected = m8_initialize(1, preferred_device);
-    if (m8_connected == 0) {
-      return 1;
-    }
+static void cleanup_resources(const unsigned char device_connected, const config_params_s *conf) {
+  if (conf->audio_enabled) {
+    audio_close();
   }
+  gamecontrollers_close();
+  renderer_close();
+  inline_font_close();
+  if (device_connected) {
+    m8_close();
+  }
+  SDL_Quit();
+  SDL_Log("Shutting down.");
+}
 
-  // initialize all SDL systems
-  if (renderer_initialize(conf.init_fullscreen) == false) {
-    SDL_Quit();
-    return 1;
+static unsigned char handle_device_initialization(unsigned char wait_for_device, const char *preferred_device) {
+  const unsigned char device_connected = m8_initialize(1, preferred_device);
+  if (!wait_for_device && device_connected == 0) {
+    SDL_LogCritical(SDL_LOG_CATEGORY_ERROR, "Device not detected!");
+    exit(EXIT_FAILURE);
   }
-  app_state = QUIT;
+  return device_connected;
+}
 
-  // initial scan for (existing) gamepads
-  gamecontrollers_initialize();
+static void main_loop(config_params_s *conf, const char *preferred_device) {
+  unsigned char device_connected = 0;
 
-#ifndef NDEBUG
-  SDL_SetLogPriorities(SDL_LOG_PRIORITY_DEBUG);
-  SDL_LogDebug(SDL_LOG_CATEGORY_TEST, "Running a Debug build");
-#endif
-
-  // main loop begin
   do {
-    // try to init connection to M8
-    m8_connected = m8_initialize(1, preferred_device);
-    if (m8_connected == 1 && m8_enable_and_reset_display() == 1) {
-      if (conf.audio_enabled == 1) {
-        audio_initialize(conf.audio_device_name, conf.audio_buffer_size);
-        // if audio is enabled, reset the display for second time to avoid glitches
-        m8_reset_display();
+    device_connected = m8_initialize(1, preferred_device);
+    if (device_connected && m8_enable_and_reset_display()) {
+      if (conf->audio_enabled) {
+        audio_initialize(conf->audio_device_name, conf->audio_buffer_size);
+        m8_reset_display();  // Avoid display glitches.
       }
       app_state = RUN;
     } else {
-      SDL_LogCritical(SDL_LOG_CATEGORY_ERROR, "Device not detected on initial check.");
-      if (conf.wait_for_device == 1) {
-        app_state = WAIT_FOR_DEVICE;
-      } else {
-        app_state = QUIT;
-      }
+      SDL_LogCritical(SDL_LOG_CATEGORY_ERROR, "Device not detected.");
+      app_state = conf->wait_for_device ? WAIT_FOR_DEVICE : QUIT;
     }
 
-    // wait until device is connected
-    if (conf.wait_for_device == 1) {
-      do_wait_for_device(preferred_device, &m8_connected, &conf);
-    } else {
-      // classic startup behaviour, exit if device is not found
-      if (m8_connected == 0) {
-        if (conf.audio_enabled == 1) {
-          audio_close();
-        }
-        gamecontrollers_close();
-        renderer_close();
-        inline_font_close();
-        SDL_Quit();
-        return 1;
-      }
+    if (conf->wait_for_device && app_state == WAIT_FOR_DEVICE) {
+      do_wait_for_device(preferred_device, &device_connected, conf);
+    } else if (!device_connected && app_state != WAIT_FOR_DEVICE) {
+      cleanup_resources(0, conf);
+      exit(EXIT_FAILURE);
     }
 
-    // main loop
+    // Handle input, process data, and render screen while running.
     while (app_state == RUN) {
       input_process(conf, &app_state);
       const int result = m8_process_data(conf);
-      if (result == 0) {
-        // Device disconnected
-        m8_connected = 0;
+      if (result == DEVICE_DISCONNECTED) {
+        device_connected = 0;
         app_state = WAIT_FOR_DEVICE;
         audio_close();
-      } else if (result == -1) {
-        // Fatal error
+      } else if (result == DEVICE_FATAL_ERROR) {
         app_state = QUIT;
       }
       render_screen();
-      SDL_Delay(conf.idle_ms);
+      SDL_Delay(conf->idle_ms);
     }
   } while (app_state > QUIT);
-  // Main loop end
 
-  // Exit, clean up
-  SDL_Log("Shutting down");
-  audio_close();
-  gamecontrollers_close();
-  renderer_close();
-  if (m8_connected == 1)
-    m8_close();
-  SDL_Quit();
-  return 0;
+  cleanup_resources(device_connected, conf);
+}
+
+
+int main(const int argc, char *argv[]) {
+  char *preferred_device = NULL;
+  char *config_filename = NULL;
+
+  config_params_s conf = initialize_config(argc, argv, &preferred_device, &config_filename);
+  initialize_signals();
+
+  const unsigned char initial_device_connected = handle_device_initialization(conf.wait_for_device, preferred_device);
+  if (!renderer_initialize(conf.init_fullscreen)) {
+    SDL_LogCritical(SDL_LOG_CATEGORY_ERROR, "Failed to initialize renderer.");
+    cleanup_resources(initial_device_connected, &conf);
+    return EXIT_FAILURE;
+  }
+
+  gamecontrollers_initialize();
+
+#ifndef NDEBUG
+  SDL_SetLogPriorities(SDL_LOG_PRIORITY_DEBUG);
+  SDL_LogDebug(SDL_LOG_CATEGORY_TEST, "Running a Debug build");
+#endif
+
+  main_loop(&conf, preferred_device);
+  return EXIT_SUCCESS;
 }
--