Skip to content

improvements to the USB communication and configuration handling in the backend and initial add of config maker #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jrmfrbg wants to merge 1 commit into
base: main
Choose a base branch
from
Open

improvements to the USB communication and configuration handling in the backend and initial add of config maker #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions backend/driver/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import (
"log"
httphandler "modularMidiGoApp/backend/httpHandler"
midiOutputPipeline "modularMidiGoApp/backend/midiUtility/midiOutputPipeline"
usbUtility "modularMidiGoApp/backend/usbUtility"

//usbUtility "modularMidiGoApp/backend/usbUtility"
"strings"
)

// Executes first and prepares:
// - Starts HTTP handler
func main() {
go midiOutputPipeline.MidiWriter()
go usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)
//go usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)

go func() {
routes := []httphandler.Route{
Expand All @@ -21,6 +22,7 @@ func main() {
httphandler.MidiTester,
httphandler.MidiPortList,
httphandler.WritePinConfig,
httphandler.StartNormalMode,
// Add more routes
}
port := parsePort(LoadHTTPconf())
Expand Down
30 changes: 16 additions & 14 deletions backend/espConfigUtility/writePinConf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package espconfigutility

import (
"bytes"
"encoding/json"
"fmt"
"os"
"path/filepath"
"strconv"

getvalues "modularMidiGoApp/backend/getValues"
usbUtility "modularMidiGoApp/backend/usbUtility"
Expand All @@ -26,7 +28,6 @@ type Multiplexer8bit struct {
}

func WritePinConfig() error {
usbUtility.StopESP32MidiListener()
fmt.Println("Writing pin configuration...")
mux, err := readConfFile()
if err != nil {
Expand All @@ -36,23 +37,24 @@ func WritePinConfig() error {

fmt.Printf("Loaded %d multiplexers from config file.\n", len(mux))

// Marshal the data and write it to the USB
for i, m := range mux {
fmt.Printf("Processing multiplexer #%d: %+v\n", i, m)
jsonData, err := json.Marshal(m)
if err != nil {
fmt.Printf("Error marshaling multiplexer #%d: %v\n", i, err)
return err
}
var configString bytes.Buffer
configString.WriteString("MUX_COUNT:" + strconv.Itoa(len(mux)) + ";")

if err := usbUtility.WriteToUSB(jsonData); err != nil {
fmt.Printf("Error writing to USB for multiplexer #%d: %v\n", i, err)
return err
for i, m := range mux {
configString.WriteString(fmt.Sprintf("MUX%d:%d,%d,%d,%d,%d",
i+1, m.Id, m.PinA, m.PinB, m.PinC, m.SerialIOpin))
for _, pin := range m.Pins {
configString.WriteString("," + strconv.Itoa(pin))
}
configString.WriteString(";")
}

fmt.Printf("JSON data for multiplexer #%d: %s\n", i, string(jsonData))
if err := usbUtility.WriteToUSB(configString.String()); err != nil {
fmt.Printf("Error writing to USB: %v\n", err)
return err
}
Comment on lines +52 to 55

@coderabbitai coderabbitai Bot Sep 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential write hang if the listener is running.

Given listenToESP32 currently holds usbMutex for its entire lifetime, this WriteToUSB call will block while the listener runs. Either:

  • Stop the listener before writing (and document this API requirement), or
  • Adopt a single owner goroutine for the serial port and submit config writes over a channel.

If choosing the minimal path, reinstate a stop before writing:

 func WritePinConfig() error {
   ...
-  if err := usbUtility.WriteToUSB(configString.String()); err != nil {
+  // Ensure exclusive USB access
+  usbUtility.StopESP32MidiListener()
+  if err := usbUtility.WriteToUSB(configString.String()); err != nil {
     fmt.Printf("Error writing to USB: %v\n", err)
     return err
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err := usbUtility.WriteToUSB(configString.String()); err != nil {
fmt.Printf("Error writing to USB: %v\n", err)
return err
}
func WritePinConfig() error {
// ... previous code ...
// Ensure exclusive USB access
usbUtility.StopESP32MidiListener()
if err := usbUtility.WriteToUSB(configString.String()); err != nil {
fmt.Printf("Error writing to USB: %v\n", err)
return err
}
// ... following code ...
}
🤖 Prompt for AI Agents 
In backend/espConfigUtility/writePinConf.go around lines 52–55, the WriteToUSB
call can hang because listenToESP32 holds usbMutex for its lifetime; stop the
listener before performing the write and restart it afterwards (or route writes
through the serial-owner goroutine). For the minimal fix: invoke the usb
listener stop function (e.g., usbUtility.StopListener or equivalent) before
calling WriteToUSB, ensure the listener has fully stopped and any mutex is
released, perform the WriteToUSB call, then restart the listener; also add a
short comment documenting that this API requires stopping the listener before
writes.


fmt.Printf("Config data sent: %s\n", configString.String())
fmt.Println("Pin configuration write completed.")
return nil
}
Expand Down Expand Up @@ -83,4 +85,4 @@ func readConfFile() ([]Multiplexer8bit, error) {

fmt.Println("Config file successfully decoded.")
return muxConfig.Multiplexers, nil
}
}
8 changes: 8 additions & 0 deletions backend/httpHandler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ type Route struct {
Handler http.HandlerFunc
}

var StartNormalMode = Route{
Path: "/startNormalMode",
Handler: func(w http.ResponseWriter, r *http.Request) {
go usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)
fmt.Fprint(w, "Normal mode started")
},
}
Comment on lines +18 to +24

@coderabbitai coderabbitai Bot Sep 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Make listener start idempotent; prevent multiple goroutines binding the same USB port.

Calling /startNormalMode repeatedly spawns overlapping ESP32MidiListener loops, likely racing the serial port despite internal mutexing.

Add a minimal in-process guard and reuse it from both start routes:

@@
-import (
+import (
 	"fmt"
+	"net/http"
+	"sync"
 	espConfigUtility "modularMidiGoApp/backend/espConfigUtility"
 	midiCCOutputer "modularMidiGoApp/backend/midiUtility"
 	midiOutputPipeline "modularMidiGoApp/backend/midiUtility/midiOutputPipeline"
 	"modularMidiGoApp/backend/usbUtility"
-	"net/http"
 )
@@
-var StartNormalMode = Route{
-	Path: "/startNormalMode",
-	Handler: func(w http.ResponseWriter, r *http.Request) {
-		go usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)
-		fmt.Fprint(w, "Normal mode started")
-	},
-}
+var (
+	listenerMu       sync.Mutex
+	listenerRunning  bool
+)
+
+func startESP32Listener(w http.ResponseWriter) {
+	listenerMu.Lock()
+	if listenerRunning {
+		http.Error(w, "Listener already running", http.StatusConflict)
+		listenerMu.Unlock()
+		return
+	}
+	listenerRunning = true
+	listenerMu.Unlock()
+
+	go func() {
+		usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)
+		listenerMu.Lock()
+		listenerRunning = false
+		listenerMu.Unlock()
+	}()
+	fmt.Fprint(w, "Normal mode started")
+}
+
+var StartNormalMode = Route{
+	Path: "/startNormalMode",
+	Handler: func(w http.ResponseWriter, r *http.Request) {
+		startESP32Listener(w)
+	},
+}

And update StartUSBListener to call startESP32Listener(w) as well.

I can also add a /stopNormalMode route that signals usbUtility’s stopChan via an exported Stop() helper for clean shutdown.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var StartNormalMode = Route{
Path: "/startNormalMode",
Handler: func(w http.ResponseWriter, r *http.Request) {
go usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)
fmt.Fprint(w, "Normal mode started")
},
}
import (
"fmt"
"net/http"
"sync"
espConfigUtility "modularMidiGoApp/backend/espConfigUtility"
midiCCOutputer "modularMidiGoApp/backend/midiUtility"
midiOutputPipeline "modularMidiGoApp/backend/midiUtility/midiOutputPipeline"
"modularMidiGoApp/backend/usbUtility"
)
var (
listenerMu sync.Mutex
listenerRunning bool
)
func startESP32Listener(w http.ResponseWriter) {
listenerMu.Lock()
if listenerRunning {
http.Error(w, "Listener already running", http.StatusConflict)
listenerMu.Unlock()
return
}
listenerRunning = true
listenerMu.Unlock()
go func() {
usbUtility.ESP32MidiListener(0, midiOutputPipeline.MidiOutChannel)
listenerMu.Lock()
listenerRunning = false
listenerMu.Unlock()
}()
fmt.Fprint(w, "Normal mode started")
}
var StartNormalMode = Route{
Path: "/startNormalMode",
Handler: func(w http.ResponseWriter, r *http.Request) {
startESP32Listener(w)
},
}
🤖 Prompt for AI Agents 
In backend/httpHandler/handler.go around lines 18-24, the current
StartNormalMode handler unconditionally spawns a new goroutine for
usbUtility.ESP32MidiListener which allows multiple concurrent listeners to bind
the same USB port; make the start idempotent by adding a small in-process guard
(e.g., a package-level boolean or sync.Once plus a mutex) and a helper function
startESP32Listener(w http.ResponseWriter) that checks the guard, starts the
listener only once (returning a suitable message if already running), and reuses
the same midiOutputPipeline.MidiOutChannel; update StartNormalMode to call that
helper and update StartUSBListener to also call startESP32Listener(w);
additionally expose a Stop() helper in usbUtility that signals its stopChan and
clear the in-process guard so the listener can be restarted or stopped cleanly.


var TestCallRoute = Route{
Path: "/testCall",
Handler: func(w http.ResponseWriter, r *http.Request) {
Expand Down
41 changes: 12 additions & 29 deletions backend/usbUtility/usb_listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"log"
midiOutputPipeline "modularMidiGoApp/backend/midiUtility/midiOutputPipeline"
"sync"
"time"

"go.bug.st/serial"
Expand All @@ -16,10 +17,12 @@ type USBPortsList struct {
SelectedUSBDevice string `json:"selected_usb_device"`
}

var usbMutex = &sync.Mutex{}
var stopChan = make(chan struct{})

func StopESP32MidiListener() {
close(stopChan)
fmt.Println("ESP32MidiListener stop signal sent.")
}
Comment on lines +20 to 26

@coderabbitai coderabbitai Bot Sep 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Stop semantics are one-shot and unsafe; double-close can panic, and restart becomes impossible.

  • Closing a package-level channel without guarding allows “close of closed channel” panics on subsequent Stop calls.
  • Once closed, a closed channel cannot be “reopened”, so future starts will immediately stop.

Minimal hardening:

 var usbMutex = &sync.Mutex{}
 var stopChan = make(chan struct{})
+var stopMu sync.Mutex

 func StopESP32MidiListener() {
-  close(stopChan)
-  fmt.Println("ESP32MidiListener stop signal sent.")
+  stopMu.Lock()
+  defer stopMu.Unlock()
+  select {
+  case <-stopChan:
+    // already closed
+  default:
+    close(stopChan)
+    fmt.Println("ESP32MidiListener stop signal sent.")
+  }
 }

Follow-up: expose a Start that reinitializes a fresh stop channel per run (or switch to context.Context).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var usbMutex = &sync.Mutex{}
var stopChan = make(chan struct{})
func StopESP32MidiListener() {
close(stopChan)
fmt.Println("ESP32MidiListener stop signal sent.")
}
var usbMutex = &sync.Mutex{}
var stopChan = make(chan struct{})
var stopMu sync.Mutex
func StopESP32MidiListener() {
stopMu.Lock()
defer stopMu.Unlock()
select {
case <-stopChan:
// already closed; nothing to do
default:
close(stopChan)
fmt.Println("ESP32MidiListener stop signal sent.")
}
}


func ESP32MidiListener(channel uint8, outputChan chan<- midiOutputPipeline.MidiCCMessage) {
Expand All @@ -36,13 +39,15 @@ func ESP32MidiListener(channel uint8, outputChan chan<- midiOutputPipeline.MidiC
return
default:
if err := listenToESP32(channel, outputChan); err != nil {
log.Printf("ESP32 connection error: %v", err)
log.Println("Retrying in 5 seconds...")
fmt.Printf("ESP32 connection error: %v", err)
fmt.Println("Retrying in 5 seconds...")

select {
case <-time.After(5 * time.Second):
continue

case <-stopChan:
fmt.Println("ESP32MidiListener stopping...")
return
}
}
Expand All @@ -51,6 +56,8 @@ func ESP32MidiListener(channel uint8, outputChan chan<- midiOutputPipeline.MidiC
}

func listenToESP32(channel uint8, outputChan chan<- midiOutputPipeline.MidiCCMessage) error {
usbMutex.Lock()
defer usbMutex.Unlock()
// Get the selected USB device
Comment on lines +59 to 61

@coderabbitai coderabbitai Bot Sep 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Global mutex held for entire listen loop blocks configuration writes (deadlock/liveness issue).

listenToESP32 acquires usbMutex and holds it while reading indefinitely; WriteToUSB also locks it, so any config write while the listener runs will block.

Options (pick one):

  • Reintroduce explicit stop around config writes (document the contract).
  • Or, refactor to a single “USB manager” goroutine that owns the port and handles both reads and serialized writes via channels (preferred).
🤖 Prompt for AI Agents 
In backend/usbUtility/usb_listener.go around lines 59-61, the current
listenToESP32 implementation locks usbMutex for the entire read loop which
blocks WriteToUSB and causes liveness/deadlock issues; refactor by creating a
single USB manager goroutine that opens and owns the serial port and serializes
access: move port open/close and the long-running read loop into that goroutine,
expose two channels (one for write requests and one for delivery of incoming
messages or events) and replace direct mutex-based WriteToUSB calls with
send-on-write-channel requests (with response/error channel if needed); ensure
the manager handles reconnection/backoff and that other code performs writes by
sending requests rather than acquiring usbMutex.

deviceName, err := GetSelectedUSBDevice(FilePath)
if err != nil {
Expand All @@ -75,19 +82,6 @@ func listenToESP32(channel uint8, outputChan chan<- midiOutputPipeline.MidiCCMes

log.Printf("Successfully connected to ESP32 on %s", deviceName)

// Wait for ESP32 to be ready for mode selection
modeSelectReader := bufio.NewReader(port)
port.SetReadTimeout(30 * time.Second)
for {
line, err := modeSelectReader.ReadString('\n')
if err != nil {
return fmt.Errorf("error reading from USB device: %w", err)
}
if line == "SELECT_MODE\n" || line == "SELECT_MODE\r\n" {
log.Println("ESP32 ready for mode selection")
break
}
}
//send mode selection command
if _, err := port.Write([]byte("BROADCAST_MODE\n")); err != nil {
return fmt.Errorf("failed to write mode selection to USB device: %w", err)
Expand Down Expand Up @@ -134,6 +128,7 @@ func processMidiData(data []byte, channel uint8, outputChan chan<- midiOutputPip

// Process pairs of bytes (CC number, value)
for i := 0; i < len(data); i += 2 {

ccNumber := data[i]
value := data[i+1]

Expand All @@ -157,6 +152,8 @@ func processMidiData(data []byte, channel uint8, outputChan chan<- midiOutputPip
}

func WriteToUSB(data interface{}) error {
usbMutex.Lock()
defer usbMutex.Unlock()
// Get the selected USB device
deviceName, err := GetSelectedUSBDevice(FilePath)
if err != nil {
Expand Down Expand Up @@ -186,20 +183,6 @@ func WriteToUSB(data interface{}) error {
return fmt.Errorf("unsupported data type for writing to USB device")
}

//Wait for ESP32 to be ready for mode selection
reader := bufio.NewReader(conn)
conn.SetReadTimeout(30 * time.Second)
for {
line, err := reader.ReadString('\n')
if err != nil {
return fmt.Errorf("error reading from USB device: %w", err)
}
if line == "SELECT_MODE\n" || line == "SELECT_MODE\r\n" {
log.Println("ESP32 ready for mode selection")
break
}
}

//send mode selection command
if _, err := conn.Write([]byte("FLASH_MODE\n")); err != nil {
return fmt.Errorf("failed to write mode selection to USB device: %w", err)
Expand Down
Loading