Conversation
…andling for multiplexers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
backend/usbUtility/usb_listener.go (1)
96-104: Configure a finite read timeout, use a typed timeout check, and honorstopChanin the inner read loop
- After opening the port, call
port.SetReadTimeout(1 * time.Second) // import "time"- In the
forloop, replace the singledefaultcase with:select { case <-stopChan: return default: }- Swap out
err.Error() == "timeout"for a proper timeout‐type check, e.g.:or useimport "errors" var te interface{ Timeout() bool } if errors.As(err, &te) && te.Timeout() { continue }errors.Is(err, os.ErrDeadlineExceeded)if you wrap deadlines.backend/espConfigUtility/writePinConf.go (1)
66-69: Path join bug: leading slash drops the root prefix.
filepath.Join(root, "/espConfigUtility/pinConf.json")ignores root on Unix because the second arg is absolute.-filePath := filepath.Join(getvalues.FindRootPath(), "/espConfigUtility/pinConf.json") +filePath := filepath.Join(getvalues.FindRootPath(), "espConfigUtility", "pinConf.json")frontend/main.go (3)
88-98: Wrong port parsed for backend API URL.parsePort currently returns http.listen_port, but generateBackendApiLocation uses it for backend API calls; this likely targets the wrong port versus http.backend_api_port.
func parsePort(unparsed string) string { var port string parts := strings.Split(unparsed, ",") for _, part := range parts { - if strings.HasPrefix(part, "listen_port:") { - port = strings.TrimPrefix(part, "listen_port:") + if strings.HasPrefix(part, "backend_api_port:") { + port = strings.TrimPrefix(part, "backend_api_port:") break } } return port }
253-265: Potential nil dereference on resp after error.On HTTP error, resp is nil but resp.Body is deferred unconditionally.
func testMidiOutput() { @@ - resp, err := http.Get(strings.Join([]string{backendApiLocation, "/testMidiOutput"}, "")) - if err != nil { - fmt.Printf("Failed to call API: %v\n", err) - } - defer resp.Body.Close() + resp, err := http.Get(strings.Join([]string{backendApiLocation, "/testMidiOutput"}, "")) + if err != nil { + fmt.Printf("Failed to call API: %v\n", err) + return + } + defer resp.Body.Close()
304-316: Same nil dereference risk in listMIDI().Return early on error before deferring on resp.
resp, err := http.Get(strings.Join([]string{backendApiLocation, "/listMidiPorts"}, "")) if err != nil { fmt.Printf("Failed to call API: %v\n", err) + return } defer resp.Body.Close()
🧹 Nitpick comments (12)
backend/usbUtility/usb_ports.json (1)
4-130: Static list likely misses real ESP32 ports; prefer runtime enumeration.ESP32 boards commonly appear as /dev/ttyUSB* or /dev/ttyACM*, which aren’t present here. Keeping a hand-maintained ttyS* list is brittle across machines.
- Generate this list at runtime via serial port enumeration (e.g., go.bug.st/serial’s port listing) and persist only the selected path.
- Keep “name” formatting consistent (e.g., either friendly names for all, or short tty names for all).
backend/usbUtility/usb_listener.go (1)
41-51: Logging consistency + newline.Mixing fmt and log and missing newline in Printf cause uneven logs.
Apply:
- fmt.Printf("ESP32 connection error: %v", err) - fmt.Println("Retrying in 5 seconds...") + log.Printf("ESP32 connection error: %v", err) + log.Println("Retrying in 5 seconds...") ... - fmt.Println("ESP32MidiListener stopping...") + log.Println("ESP32MidiListener stopping...")backend/espConfigUtility/writePinConf.go (2)
40-51: Consolidated config string looks good; add a terminator and versioning for robustness.To aid parsing and future changes, consider adding:
- A version prefix (e.g., CFG_V1;) and
- A trailing newline after the final semicolon.
- configString.WriteString("MUX_COUNT:" + strconv.Itoa(len(mux)) + ";") + configString.WriteString("CFG_V1;MUX_COUNT:" + strconv.Itoa(len(mux)) + ";") ... - configString.WriteString(";") + configString.WriteString(";") +} +// Ensure line termination for line-oriented parsers +configString.WriteString("\n")
57-57: Avoid logging full config payloads in production logs.Config may be large or sensitive; log length and hash instead.
- fmt.Printf("Config data sent: %s\n", configString.String()) + fmt.Printf("Config data sent: %d bytes\n", len(configString.String()))esp_data/modularMidiDriver/modularMidiDriver.ino (2)
120-124: Invalid config path lacks a safe fallback.If flash contains a bad config, you bail out but continue with uninitialized pins. Consider applying a minimal safe default or blocking BROADCAST mode until a valid config is written.
I can add applyDefaultConfig() that zeros pins and disables reads, plus an explicit error prompt.
125-165: Duplicate parsing logic: consolidate to a single parser.validateConfig and loadConfigFromFlash duplicate the split and indexing logic. Extract a parseConfig(String, Mux[]) function that validates and populates structs in one pass to reduce drift and bugs.
I can provide a refactor that returns bool and fills multiplexers by reference.
backend/driver/main.go (1)
19-27: Route list still includes both StartNormalMode and legacy USB listener endpoints.Having multiple “start” routes can confuse users and risks duplicate starts. Consider consolidating to one canonical route.
frontend/main.go (2)
150-152: CLI message is misleading; adjust text and add usage entry.Printing "Normal mode selected" before the API succeeds can confuse users. Change the message to indicate an attempt, and add this command to the usage text.
case "normal-mode": - fmt.Println("Normal mode selected") + fmt.Println("Starting normal mode...") enterNormalMode()Outside this hunk, update printUsage():
func printUsage() { // Print usage instructions for the CLI tool @@ - fmt.Println(" usb-manager help - Show this help message") + fmt.Println(" usb-manager help - Show this help message") + fmt.Println(" usb-manager normal-mode - Start ESP32 listener (normal mode)")
100-111: Remove leftover debug print from parseProtocol.Avoid noisy stdout in CLI tools.
func parseProtocol(unparsed string) string { @@ - fmt.Println(protocol) return protocol }configMaker/templates/index.html (3)
170-183: Use type=number with min/step for pin inputs.Improves UX and native validation; combine with dynamic max via JS.
-<input type="text" name="mux-${i}-pinA" pattern="[0-9]+" title="Enter a number" style="width: 50px;"> +<input type="number" name="mux-${i}-pinA" min="0" step="1" title="Enter a number" style="width: 50px;"> @@ -<input type="text" name="mux-${i}-pinB" pattern="[0-9]+" title="Enter a number" style="width: 50px;"> +<input type="number" name="mux-${i}-pinB" min="0" step="1" title="Enter a number" style="width: 50px;"> @@ -<input type="text" name="mux-${i}-pinC" pattern="[0-9]+" title="Enter a number" style="width: 50px;"> +<input type="number" name="mux-${i}-pinC" min="0" step="1" title="Enter a number" style="width: 50px;"> @@ -<input type="text" name="mux-${i}-serialIOpin" pattern="[0-9]+" title="Enter a number" style="width: 50px;"> +<input type="number" name="mux-${i}-serialIOpin" min="0" step="1" title="Enter a number" style="width: 50px;">
196-210: Match JS selector to number inputs and set dynamic max.After switching to number inputs, update the selector and set max from GPIO count.
-const inputs = muxElement.querySelectorAll('input[type="text"]'); +const inputs = muxElement.querySelectorAll('input[type="number"]'); inputs.forEach(input => { + input.max = gpioPinsInput.value; input.addEventListener('input', () => { const gpioCount = parseInt(gpioPinsInput.value, 10); const value = parseInt(input.value, 10); if (isNaN(value) || value < 0 || value >= gpioCount) { input.style.borderColor = 'red'; } else { input.style.borderColor = '#456387'; // Reset to default } }); // Trigger validation on load input.dispatchEvent(new Event('input')); });
7-7: Add Subresource Integrity (SRI) and crossorigin attributes to the htmx script tagReplace the existing include with the pinned version:
<script src="https://unpkg.com/htmx.org@1.9.10" integrity="sha384-D1Kt99CQMDuVetoL1lrYwg5t+9QdHe7NLX/SoJYkXDFfX37iInKRy5xLSi8nO7UC" crossorigin="anonymous"> </script>Optionally, vendor the file locally for full supply-chain control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
signal-2025-07-08-163203_002.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (9)
backend/driver/main.go(2 hunks)backend/espConfigUtility/writePinConf.go(3 hunks)backend/httpHandler/handler.go(1 hunks)backend/usbUtility/usb_listener.go(6 hunks)backend/usbUtility/usb_ports.json(1 hunks)configMaker/main.go(1 hunks)configMaker/templates/index.html(1 hunks)esp_data/modularMidiDriver/modularMidiDriver.ino(4 hunks)frontend/main.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
esp_data/**/*.ino
📄 CodeRabbit inference engine (esp_data/GEMINI.md)
esp_data/**/*.ino: Include Arduino.h in the Arduino sketch
Declare sensor pin assignments using #define or const int
Maintain state (e.g., array/struct) of last-known sensor values to detect significant changes
In setup(), initialize Serial at the same baud rate as the backend (e.g., 9600)
Configure pinMode for each sensor (INPUT for potentiometers; INPUT_PULLUP for buttons) in setup()
In loop(), read all sensors continuously and only send data when value changes exceed a defined threshold
Map ESP32 ADC readings (0–4095) to MIDI range (0–127) for potentiometers
Send MIDI Control Change data as two bytes (controller, value) followed by a newline over Serial
Provide a helper function sendMidiData(uint8_t controller, uint8_t value) that writes the two bytes then a newline
For buttons using INPUT_PULLUP, on transition to LOW send value 127; on transition to HIGH send value 0; update last state
Use a mapping structure (e.g., array of structs) to associate each input pin with a specific MIDI CC number
Terminate each packet (one or more (controller,value) pairs) with a newline character to match backend protocol
Files:
esp_data/modularMidiDriver/modularMidiDriver.ino
🧠 Learnings (2)
📚 Learning: 2025-08-26T20:18:54.330Z
Learnt from: CR
PR: jrmfrbg/modularMidiDriver#0
File: esp_data/GEMINI.md:0-0
Timestamp: 2025-08-26T20:18:54.330Z
Learning: Applies to esp_data/**/*.ino : Use a mapping structure (e.g., array of structs) to associate each input pin with a specific MIDI CC number
Applied to files:
esp_data/modularMidiDriver/modularMidiDriver.ino
📚 Learning: 2025-08-26T20:18:54.330Z
Learnt from: CR
PR: jrmfrbg/modularMidiDriver#0
File: esp_data/GEMINI.md:0-0
Timestamp: 2025-08-26T20:18:54.330Z
Learning: Applies to esp_data/**/*.ino : In setup(), initialize Serial at the same baud rate as the backend (e.g., 9600)
Applied to files:
esp_data/modularMidiDriver/modularMidiDriver.ino
🧬 Code graph analysis (4)
backend/espConfigUtility/writePinConf.go (1)
backend/usbUtility/usb_listener.go (1)
WriteToUSB(154-197)
backend/httpHandler/handler.go (2)
backend/usbUtility/usb_listener.go (1)
ESP32MidiListener(28-56)backend/midiUtility/midiOutputPipeline/outputer.go (1)
MidiOutChannel(38-38)
backend/usbUtility/usb_ports.json (2)
frontend-gui/main.go (1)
AvailableUSBDevices(29-32)backend/usbUtility/usbPortLists.go (1)
Name(16-19)
backend/driver/main.go (2)
backend/midiUtility/midiOutputPipeline/outputer.go (1)
MidiWriter(40-82)backend/httpHandler/handler.go (1)
StartNormalMode(18-24)
🔇 Additional comments (3)
backend/usbUtility/usb_listener.go (1)
85-89: Mode switching protocol: verify device returns to normal mode post-write.Listener enters BROADCAST_MODE; writer enters FLASH_MODE and exits without restoring. Ensure the firmware returns to broadcast/normal on its own, or send an explicit “exit”/“mode restore” to avoid the device staying in flash mode.
Also applies to: 186-194
esp_data/modularMidiDriver/modularMidiDriver.ino (1)
17-18: Confirm Serial2 pin mapping for your ESP32 board.Pins 11/10 may not exist or may be mapped differently on some ESP32 variants. Consider making TX2/RX2 configurable (via config or board profile) or defaulting to known-good pins for your board.
backend/driver/main.go (1)
15-17: Deferring ESP32 listener startup to the API is fine.Commenting out the auto-start aligns with the new /startNormalMode route and reduces surprise on boot.
Ensure the frontend calls /startNormalMode on demand, and document this in README.
| if err := usbUtility.WriteToUSB(configString.String()); err != nil { | ||
| fmt.Printf("Error writing to USB: %v\n", err) | ||
| return err | ||
| } |
There was a problem hiding this comment.
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.
| 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.
| 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") | ||
| }, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
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.
| 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 usbMutex = &sync.Mutex{} | ||
| var stopChan = make(chan struct{}) | ||
|
|
||
| func StopESP32MidiListener() { | ||
| close(stopChan) | ||
| fmt.Println("ESP32MidiListener stop signal sent.") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
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.
| 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.") | |
| } | |
| } |
| usbMutex.Lock() | ||
| defer usbMutex.Unlock() | ||
| // Get the selected USB device |
There was a problem hiding this comment.
🛠️ Refactor suggestion
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.
| "name": "ttyS9" | ||
| } | ||
| ], | ||
| "selected_usb_device": "/dev/ttyACM0" |
There was a problem hiding this comment.
selected_usb_device not present in available list (UI/boot failure risk).
/dev/ttyACM0 isn’t listed under available_usb_devices. Any code that validates or opens the selected device will fail.
Apply a concrete fix by selecting an existing path (example shown):
- "selected_usb_device": "/dev/ttyACM0"
+ "selected_usb_device": "/dev/ttyS4"If dynamic discovery exists, consider auto-selecting the first available entry or persisting the last known-good path instead.
📝 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.
| "selected_usb_device": "/dev/ttyACM0" | |
| "selected_usb_device": "/dev/ttyS4" |
🤖 Prompt for AI Agents
In backend/usbUtility/usb_ports.json around line 132, the "selected_usb_device"
value ("/dev/ttyACM0") is not present in the available_usb_devices list which
will cause validation/opening failures; fix by replacing "selected_usb_device"
with one of the actual paths that exists in available_usb_devices (or, if
discovery is dynamic, implement logic to auto-select the first available device
or persist the last known-good device when the configured entry is missing), and
ensure persistence/update of usb_ports.json so the selected path always matches
an entry in available_usb_devices.
| func handleGenerate(w http.ResponseWriter, r *http.Request) { | ||
| if err := r.ParseForm(); err != nil { | ||
| http.Error(w, "Failed to parse form", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| muxCountStr := r.FormValue("mux-count") | ||
| muxCount, err := strconv.Atoi(muxCountStr) | ||
| if err != nil { | ||
| http.Error(w, "Invalid number of multiplexers", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| config := PinConfig{} | ||
|
|
||
| for i := 0; i < muxCount; i++ { | ||
| muxIDStr := r.FormValue("mux-" + strconv.Itoa(i) + "-id") | ||
| muxID, _ := strconv.Atoi(muxIDStr) | ||
|
|
||
| pinAStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinA") | ||
| pinA, _ := strconv.Atoi(pinAStr) | ||
|
|
||
| pinBStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinB") | ||
| pinB, _ := strconv.Atoi(pinBStr) | ||
|
|
||
| pinCStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinC") | ||
| pinC, _ := strconv.Atoi(pinCStr) | ||
|
|
||
| serialIOpinStr := r.FormValue("mux-" + strconv.Itoa(i) + "-serialIOpin") | ||
| serialIOpin, _ := strconv.Atoi(serialIOpinStr) | ||
|
|
||
| muxPins := make([]int, 8) | ||
| for j := 0; j < 8; j++ { | ||
| pinValueStr := r.FormValue("mux-" + strconv.Itoa(i) + "-io-" + strconv.Itoa(j)) | ||
| muxPins[j], _ = strconv.Atoi(pinValueStr) | ||
| } | ||
|
|
||
| mux := MultiplexerConfig{ | ||
| ID: muxID, | ||
| Pins: muxPins, | ||
| PinA: pinA, | ||
| PinB: pinB, | ||
| PinC: pinC, | ||
| SerialIOpin: serialIOpin, | ||
| } | ||
| config.Multiplexers = append(config.Multiplexers, mux) | ||
| } | ||
|
|
||
| jsonData, err := json.MarshalIndent(config, "", " ") | ||
| if err != nil { | ||
| http.Error(w, "Failed to generate JSON", http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| w.Header().Set("Content-Type", "application/json") | ||
| w.Write(jsonData) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate all numeric inputs; don’t ignore Atoi errors.
Currently all Atoi errors are dropped, defaulting to zero—this can generate corrupt configs. Validate muxCount range and each per-mux field; fail fast with 400.
func handleGenerate(w http.ResponseWriter, r *http.Request) {
@@
- muxCountStr := r.FormValue("mux-count")
- muxCount, err := strconv.Atoi(muxCountStr)
+ muxCountStr := r.FormValue("mux-count")
+ muxCount, err := strconv.Atoi(muxCountStr)
if err != nil {
http.Error(w, "Invalid number of multiplexers", http.StatusBadRequest)
return
}
+ if muxCount < 1 || muxCount > 10 {
+ http.Error(w, "Multiplexer count must be between 1 and 10", http.StatusBadRequest)
+ return
+ }
@@
- for i := 0; i < muxCount; i++ {
+ for i := 0; i < muxCount; i++ {
muxIDStr := r.FormValue("mux-" + strconv.Itoa(i) + "-id")
- muxID, _ := strconv.Atoi(muxIDStr)
+ muxID, err := strconv.Atoi(muxIDStr)
+ if err != nil || muxID <= 0 {
+ http.Error(w, "Invalid mux id at index "+strconv.Itoa(i), http.StatusBadRequest)
+ return
+ }
pinAStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinA")
- pinA, _ := strconv.Atoi(pinAStr)
+ pinA, err := strconv.Atoi(pinAStr)
+ if err != nil || pinA < 0 {
+ http.Error(w, "Invalid pinA for mux "+strconv.Itoa(i), http.StatusBadRequest)
+ return
+ }
pinBStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinB")
- pinB, _ := strconv.Atoi(pinBStr)
+ pinB, err := strconv.Atoi(pinBStr)
+ if err != nil || pinB < 0 {
+ http.Error(w, "Invalid pinB for mux "+strconv.Itoa(i), http.StatusBadRequest)
+ return
+ }
pinCStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinC")
- pinC, _ := strconv.Atoi(pinCStr)
+ pinC, err := strconv.Atoi(pinCStr)
+ if err != nil || pinC < 0 {
+ http.Error(w, "Invalid pinC for mux "+strconv.Itoa(i), http.StatusBadRequest)
+ return
+ }
serialIOpinStr := r.FormValue("mux-" + strconv.Itoa(i) + "-serialIOpin")
- serialIOpin, _ := strconv.Atoi(serialIOpinStr)
+ serialIOpin, err := strconv.Atoi(serialIOpinStr)
+ if err != nil || serialIOpin < 0 {
+ http.Error(w, "Invalid serialIOpin for mux "+strconv.Itoa(i), http.StatusBadRequest)
+ return
+ }
muxPins := make([]int, 8)
for j := 0; j < 8; j++ {
pinValueStr := r.FormValue("mux-" + strconv.Itoa(i) + "-io-" + strconv.Itoa(j))
- muxPins[j], _ = strconv.Atoi(pinValueStr)
+ v, err := strconv.Atoi(pinValueStr)
+ if err != nil || v < 1 || v > 3 {
+ http.Error(w, "Invalid IO type for mux "+strconv.Itoa(i)+" pin "+strconv.Itoa(j), http.StatusBadRequest)
+ return
+ }
+ muxPins[j] = v
}📝 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.
| func handleGenerate(w http.ResponseWriter, r *http.Request) { | |
| if err := r.ParseForm(); err != nil { | |
| http.Error(w, "Failed to parse form", http.StatusBadRequest) | |
| return | |
| } | |
| muxCountStr := r.FormValue("mux-count") | |
| muxCount, err := strconv.Atoi(muxCountStr) | |
| if err != nil { | |
| http.Error(w, "Invalid number of multiplexers", http.StatusBadRequest) | |
| return | |
| } | |
| config := PinConfig{} | |
| for i := 0; i < muxCount; i++ { | |
| muxIDStr := r.FormValue("mux-" + strconv.Itoa(i) + "-id") | |
| muxID, _ := strconv.Atoi(muxIDStr) | |
| pinAStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinA") | |
| pinA, _ := strconv.Atoi(pinAStr) | |
| pinBStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinB") | |
| pinB, _ := strconv.Atoi(pinBStr) | |
| pinCStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinC") | |
| pinC, _ := strconv.Atoi(pinCStr) | |
| serialIOpinStr := r.FormValue("mux-" + strconv.Itoa(i) + "-serialIOpin") | |
| serialIOpin, _ := strconv.Atoi(serialIOpinStr) | |
| muxPins := make([]int, 8) | |
| for j := 0; j < 8; j++ { | |
| pinValueStr := r.FormValue("mux-" + strconv.Itoa(i) + "-io-" + strconv.Itoa(j)) | |
| muxPins[j], _ = strconv.Atoi(pinValueStr) | |
| } | |
| mux := MultiplexerConfig{ | |
| ID: muxID, | |
| Pins: muxPins, | |
| PinA: pinA, | |
| PinB: pinB, | |
| PinC: pinC, | |
| SerialIOpin: serialIOpin, | |
| } | |
| config.Multiplexers = append(config.Multiplexers, mux) | |
| } | |
| jsonData, err := json.MarshalIndent(config, "", " ") | |
| if err != nil { | |
| http.Error(w, "Failed to generate JSON", http.StatusInternalServerError) | |
| return | |
| } | |
| w.Header().Set("Content-Type", "application/json") | |
| w.Write(jsonData) | |
| } | |
| func handleGenerate(w http.ResponseWriter, r *http.Request) { | |
| if err := r.ParseForm(); err != nil { | |
| http.Error(w, "Failed to parse form", http.StatusBadRequest) | |
| return | |
| } | |
| muxCountStr := r.FormValue("mux-count") | |
| muxCount, err := strconv.Atoi(muxCountStr) | |
| if err != nil { | |
| http.Error(w, "Invalid number of multiplexers", http.StatusBadRequest) | |
| return | |
| } | |
| if muxCount < 1 || muxCount > 10 { | |
| http.Error(w, "Multiplexer count must be between 1 and 10", http.StatusBadRequest) | |
| return | |
| } | |
| config := PinConfig{} | |
| for i := 0; i < muxCount; i++ { | |
| muxIDStr := r.FormValue("mux-" + strconv.Itoa(i) + "-id") | |
| muxID, err := strconv.Atoi(muxIDStr) | |
| if err != nil || muxID <= 0 { | |
| http.Error(w, "Invalid mux id at index "+strconv.Itoa(i), http.StatusBadRequest) | |
| return | |
| } | |
| pinAStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinA") | |
| pinA, err := strconv.Atoi(pinAStr) | |
| if err != nil || pinA < 0 { | |
| http.Error(w, "Invalid pinA for mux "+strconv.Itoa(i), http.StatusBadRequest) | |
| return | |
| } | |
| pinBStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinB") | |
| pinB, err := strconv.Atoi(pinBStr) | |
| if err != nil || pinB < 0 { | |
| http.Error(w, "Invalid pinB for mux "+strconv.Itoa(i), http.StatusBadRequest) | |
| return | |
| } | |
| pinCStr := r.FormValue("mux-" + strconv.Itoa(i) + "-pinC") | |
| pinC, err := strconv.Atoi(pinCStr) | |
| if err != nil || pinC < 0 { | |
| http.Error(w, "Invalid pinC for mux "+strconv.Itoa(i), http.StatusBadRequest) | |
| return | |
| } | |
| serialIOpinStr := r.FormValue("mux-" + strconv.Itoa(i) + "-serialIOpin") | |
| serialIOpin, err := strconv.Atoi(serialIOpinStr) | |
| if err != nil || serialIOpin < 0 { | |
| http.Error(w, "Invalid serialIOpin for mux "+strconv.Itoa(i), http.StatusBadRequest) | |
| return | |
| } | |
| muxPins := make([]int, 8) | |
| for j := 0; j < 8; j++ { | |
| pinValueStr := r.FormValue("mux-" + strconv.Itoa(i) + "-io-" + strconv.Itoa(j)) | |
| v, err := strconv.Atoi(pinValueStr) | |
| if err != nil || v < 1 || v > 3 { | |
| http.Error(w, "Invalid IO type for mux "+strconv.Itoa(i)+" pin "+strconv.Itoa(j), http.StatusBadRequest) | |
| return | |
| } | |
| muxPins[j] = v | |
| } | |
| mux := MultiplexerConfig{ | |
| ID: muxID, | |
| Pins: muxPins, | |
| PinA: pinA, | |
| PinB: pinB, | |
| PinC: pinC, | |
| SerialIOpin: serialIOpin, | |
| } | |
| config.Multiplexers = append(config.Multiplexers, mux) | |
| } | |
| jsonData, err := json.MarshalIndent(config, "", " ") | |
| if err != nil { | |
| http.Error(w, "Failed to generate JSON", http.StatusInternalServerError) | |
| return | |
| } | |
| w.Header().Set("Content-Type", "application/json") | |
| w.Write(jsonData) | |
| } |
🤖 Prompt for AI Agents
In configMaker/main.go around lines 35 to 91 the code silently ignores
strconv.Atoi errors for muxCount, per-mux IDs, pins and the 8 mux IO entries
(defaulting to zero), and does not validate muxCount range; update the handler
to check and return http.StatusBadRequest on any Atoi error for muxCount and
every per-mux numeric field (mux ID, pinA, pinB, pinC, serialIOpin and each of
the 8 io values), validate muxCount is within a sane range (e.g., >=0 and a
reasonable max), ensure the muxPins slice is exactly 8 and that each value
parsed correctly, and stop processing and respond with a clear 400 error message
when any validation/parsing fails (fail fast).
| bool validateConfig(String configString) { | ||
| if (configString == "") { | ||
| return false; | ||
| } | ||
|
|
||
| // Parse MUX_COUNT | ||
| int muxCountIndex = configString.indexOf("MUX_COUNT:"); | ||
| if (muxCountIndex == -1) { | ||
| return false; | ||
| } | ||
| int muxCount = configString.substring(muxCountIndex + 10).toInt(); | ||
| if (muxCount > MUX_COUNT) { | ||
| muxCount = MUX_COUNT; | ||
| } | ||
|
|
||
| // Parse each multiplexer's data | ||
| int startIndex = 0; | ||
| for (int i = 0; i < muxCount; i++) { | ||
| String muxKey = "MUX" + String(i + 1) + ":"; | ||
| int muxIndex = configString.indexOf(muxKey, startIndex); | ||
| if (muxIndex == -1) { | ||
| return false; | ||
| } | ||
| int endIndex = configString.indexOf(';', muxIndex); | ||
| if (endIndex == -1) { | ||
| endIndex = configString.length(); | ||
| } | ||
| String muxData = configString.substring(muxIndex + muxKey.length(), endIndex); | ||
|
|
||
| // Split the muxData by commas | ||
| int valueIndex = 0; | ||
| int lastIndex = 0; | ||
| String values[13]; | ||
| for(int j = 0; j < 13; j++) { | ||
| int currentIndex = muxData.indexOf(',', lastIndex); | ||
| if (currentIndex == -1) { | ||
| currentIndex = muxData.length(); | ||
| } | ||
| values[j] = muxData.substring(lastIndex, currentIndex); | ||
| lastIndex = currentIndex + 1; | ||
| } | ||
|
|
||
| if (lastIndex < muxData.length()) { | ||
| return false; | ||
| } | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Config validation can accept malformed MUX entries (fewer than 13 fields).
The current split loop always assigns 13 values, so missing fields become empty strings (toInt => 0). This falsely validates bad configs and can drive pin 0, risking hardware mis-wiring.
Enforce exact field count and robust MUX_COUNT parsing:
bool validateConfig(String configString) {
if (configString == "") {
return false;
}
// Parse MUX_COUNT
- int muxCountIndex = configString.indexOf("MUX_COUNT:");
- if (muxCountIndex == -1) {
- return false;
- }
- int muxCount = configString.substring(muxCountIndex + 10).toInt();
+ int muxCountIndex = configString.indexOf("MUX_COUNT:");
+ if (muxCountIndex == -1) {
+ return false;
+ }
+ int muxCountEnd = configString.indexOf(';', muxCountIndex);
+ if (muxCountEnd == -1) {
+ return false;
+ }
+ int muxCount = configString.substring(muxCountIndex + 10, muxCountEnd).toInt();
if (muxCount > MUX_COUNT) {
muxCount = MUX_COUNT;
}
// Parse each multiplexer's data
int startIndex = 0;
for (int i = 0; i < muxCount; i++) {
String muxKey = "MUX" + String(i + 1) + ":";
int muxIndex = configString.indexOf(muxKey, startIndex);
if (muxIndex == -1) {
return false;
}
int endIndex = configString.indexOf(';', muxIndex);
if (endIndex == -1) {
endIndex = configString.length();
}
String muxData = configString.substring(muxIndex + muxKey.length(), endIndex);
-
- // Split the muxData by commas
- int valueIndex = 0;
- int lastIndex = 0;
- String values[13];
- for(int j = 0; j < 13; j++) {
- int currentIndex = muxData.indexOf(',', lastIndex);
- if (currentIndex == -1) {
- currentIndex = muxData.length();
- }
- values[j] = muxData.substring(lastIndex, currentIndex);
- lastIndex = currentIndex + 1;
- }
-
- if (lastIndex < muxData.length()) {
- return false;
- }
+ // Require exactly 13 fields (12 commas) and no empty tokens
+ int commaCount = 0;
+ for (int k = 0; k < muxData.length(); ++k) {
+ if (muxData.charAt(k) == ',') commaCount++;
+ }
+ if (commaCount != 12) {
+ return false;
+ }
+ int lastIndex = 0;
+ String values[13];
+ for (int j = 0; j < 13; j++) {
+ int currentIndex = muxData.indexOf(',', lastIndex);
+ if (currentIndex == -1) currentIndex = muxData.length();
+ if (currentIndex == lastIndex) { // empty token
+ return false;
+ }
+ values[j] = muxData.substring(lastIndex, currentIndex);
+ lastIndex = currentIndex + 1;
+ }
+ if (lastIndex != muxData.length() + 1) {
+ return false;
+ }
}
return true;
}📝 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.
| bool validateConfig(String configString) { | |
| if (configString == "") { | |
| return false; | |
| } | |
| // Parse MUX_COUNT | |
| int muxCountIndex = configString.indexOf("MUX_COUNT:"); | |
| if (muxCountIndex == -1) { | |
| return false; | |
| } | |
| int muxCount = configString.substring(muxCountIndex + 10).toInt(); | |
| if (muxCount > MUX_COUNT) { | |
| muxCount = MUX_COUNT; | |
| } | |
| // Parse each multiplexer's data | |
| int startIndex = 0; | |
| for (int i = 0; i < muxCount; i++) { | |
| String muxKey = "MUX" + String(i + 1) + ":"; | |
| int muxIndex = configString.indexOf(muxKey, startIndex); | |
| if (muxIndex == -1) { | |
| return false; | |
| } | |
| int endIndex = configString.indexOf(';', muxIndex); | |
| if (endIndex == -1) { | |
| endIndex = configString.length(); | |
| } | |
| String muxData = configString.substring(muxIndex + muxKey.length(), endIndex); | |
| // Split the muxData by commas | |
| int valueIndex = 0; | |
| int lastIndex = 0; | |
| String values[13]; | |
| for(int j = 0; j < 13; j++) { | |
| int currentIndex = muxData.indexOf(',', lastIndex); | |
| if (currentIndex == -1) { | |
| currentIndex = muxData.length(); | |
| } | |
| values[j] = muxData.substring(lastIndex, currentIndex); | |
| lastIndex = currentIndex + 1; | |
| } | |
| if (lastIndex < muxData.length()) { | |
| return false; | |
| } | |
| } | |
| return true; | |
| } | |
| bool validateConfig(String configString) { | |
| if (configString == "") { | |
| return false; | |
| } | |
| // Parse MUX_COUNT | |
| int muxCountIndex = configString.indexOf("MUX_COUNT:"); | |
| if (muxCountIndex == -1) { | |
| return false; | |
| } | |
| int muxCountEnd = configString.indexOf(';', muxCountIndex); | |
| if (muxCountEnd == -1) { | |
| return false; | |
| } | |
| int muxCount = configString.substring(muxCountIndex + 10, muxCountEnd).toInt(); | |
| if (muxCount > MUX_COUNT) { | |
| muxCount = MUX_COUNT; | |
| } | |
| // Parse each multiplexer's data | |
| int startIndex = 0; | |
| for (int i = 0; i < muxCount; i++) { | |
| String muxKey = "MUX" + String(i + 1) + ":"; | |
| int muxIndex = configString.indexOf(muxKey, startIndex); | |
| if (muxIndex == -1) { | |
| return false; | |
| } | |
| int endIndex = configString.indexOf(';', muxIndex); | |
| if (endIndex == -1) { | |
| endIndex = configString.length(); | |
| } | |
| String muxData = configString.substring(muxIndex + muxKey.length(), endIndex); | |
| // Require exactly 13 fields (12 commas) and no empty tokens | |
| int commaCount = 0; | |
| for (int k = 0; k < muxData.length(); ++k) { | |
| if (muxData.charAt(k) == ',') commaCount++; | |
| } | |
| if (commaCount != 12) { | |
| return false; | |
| } | |
| int lastIndex = 0; | |
| String values[13]; | |
| for (int j = 0; j < 13; j++) { | |
| int currentIndex = muxData.indexOf(',', lastIndex); | |
| if (currentIndex == -1) { | |
| currentIndex = muxData.length(); | |
| } | |
| if (currentIndex == lastIndex) { // empty token | |
| return false; | |
| } | |
| values[j] = muxData.substring(lastIndex, currentIndex); | |
| lastIndex = currentIndex + 1; | |
| } | |
| if (lastIndex != muxData.length() + 1) { | |
| return false; | |
| } | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In esp_data/modularMidiDriver/modularMidiDriver.ino around lines 68-115, the
validator currently always extracts 13 fields per MUX entry even when fewer
exist and uses substring.indexOf from startIndex 0 which can mis-locate
subsequent MUX entries; update validation to (1) parse MUX_COUNT robustly:
ensure the "MUX_COUNT:" key exists, extract only up to the next
separator/semicolon, trim and toInt, and validate it's between 1 and MUX_COUNT
constant (clamp or fail if out of range); (2) when locating each "MUXn:" use
startIndex and after finding endIndex set startIndex = endIndex + 1 so next
search begins after the processed entry; (3) split muxData by commas but verify
the exact number of fields is 13 (don’t prefill an array of 13 blindly), e.g.
count separators or loop collecting values until no more commas and then check
count == 13; (4) ensure no required field is empty (trim and reject empty
strings) and only then call toInt; if any check fails return false.
| void setup() { | ||
| Serial.begin(9600); | ||
|
|
||
| sleep(1); | ||
| Serial.println("STARTING UP! Serial1"); | ||
| Serial2.begin(9600, SERIAL_8N1, RX2, TX2); | ||
| sleep(1); | ||
| Serial2.println("STARTING UP! Serial2"); | ||
| sleep(1); | ||
| Serial.println("STARTED UP!"); | ||
| // Load configuration from flash memory |
There was a problem hiding this comment.
Use Arduino delay(), not sleep().
sleep(1) is not an Arduino API and will not compile under Arduino core. Replace with delay(1000).
- sleep(1);
+ delay(1000);
Serial.println("STARTING UP! Serial1");
- Serial2.begin(9600, SERIAL_8N1, RX2, TX2);
- sleep(1);
+ Serial2.begin(9600, SERIAL_8N1, RX2, TX2);
+ delay(1000);
Serial2.println("STARTING UP! Serial2");
- sleep(1);
+ delay(1000);
Serial.println("STARTED UP!");📝 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.
| void setup() { | |
| Serial.begin(9600); | |
| sleep(1); | |
| Serial.println("STARTING UP! Serial1"); | |
| Serial2.begin(9600, SERIAL_8N1, RX2, TX2); | |
| sleep(1); | |
| Serial2.println("STARTING UP! Serial2"); | |
| sleep(1); | |
| Serial.println("STARTED UP!"); | |
| // Load configuration from flash memory | |
| void setup() { | |
| Serial.begin(9600); | |
| delay(1000); | |
| Serial.println("STARTING UP! Serial1"); | |
| Serial2.begin(9600, SERIAL_8N1, RX2, TX2); | |
| delay(1000); | |
| Serial2.println("STARTING UP! Serial2"); | |
| delay(1000); | |
| Serial.println("STARTED UP!"); | |
| // Load configuration from flash memory |
🤖 Prompt for AI Agents
In esp_data/modularMidiDriver/modularMidiDriver.ino around lines 177 to 186, the
code calls sleep(1) which is not part of the Arduino API and will not compile;
replace each sleep(1) with delay(1000) (delay takes milliseconds) so the startup
waits one second between serial init/prints; update all occurrences in this
block accordingly.
| if (validateConfig(input)) { | ||
| saveConfigToFlash(input); | ||
| Serial.println("Config saved!"); | ||
| } else { | ||
| Serial.println("Invalid config format!"); | ||
| } | ||
| } else { | ||
| Serial.println("Unknown command in BROADCAST mode."); | ||
| } |
There was a problem hiding this comment.
No sensor polling in BROADCAST mode.
loop() never calls readSensors(), so nothing is emitted in BROADCAST. This breaks the MIDI stream.
Add continuous polling when in BROADCAST (outside the Serial.available() branch):
// place after the closing brace of the Serial.available() block
if (currentMode == BROADCAST) {
readSensors();
delay(2); // small throttle
}Also consider suppressing “Unknown command in BROADCAST mode.” to avoid spurious logs while streaming.
🤖 Prompt for AI Agents
In esp_data/modularMidiDriver/modularMidiDriver.ino around lines 217 to 225,
loop() currently handles Serial.available() commands but never polls sensors in
BROADCAST mode; add a block after the Serial.available() branch that checks if
currentMode == BROADCAST, calls readSensors(), and delays briefly (e.g.,
delay(2)) to continuously emit MIDI data. Also mute or remove the “Unknown
command in BROADCAST mode.” Serial.println when currentMode == BROADCAST to
avoid spurious logs during streaming.
| func enterNormalMode() { | ||
| resp, err := http.Get(strings.Join([]string{backendApiLocation, "/startNormalMode"}, "")) | ||
| if err != nil { | ||
| fmt.Printf("Failed to call API: %v\n", err) | ||
| return | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| fmt.Printf("API returned status code: %d\n", resp.StatusCode) | ||
| return | ||
| } | ||
|
|
||
| fmt.Println("Entered normal mode successfully.") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout and context for the HTTP call.
Use a context with timeout to avoid hanging if the backend is unreachable; also handle request build errors.
func enterNormalMode() {
- resp, err := http.Get(strings.Join([]string{backendApiLocation, "/startNormalMode"}, ""))
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+ req, err := http.NewRequestWithContext(ctx, http.MethodGet, backendApiLocation+"/startNormalMode", nil)
+ if err != nil {
+ fmt.Printf("Failed to build request: %v\n", err)
+ return
+ }
+ resp, err := http.DefaultClient.Do(req)
if err != nil {
fmt.Printf("Failed to call API: %v\n", err)
return
}
defer resp.Body.Close()
@@
fmt.Println("Entered normal mode successfully.")
}Add imports:
import (
+ "context"
@@
"strings"
+ "time"📝 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.
| func enterNormalMode() { | |
| resp, err := http.Get(strings.Join([]string{backendApiLocation, "/startNormalMode"}, "")) | |
| if err != nil { | |
| fmt.Printf("Failed to call API: %v\n", err) | |
| return | |
| } | |
| defer resp.Body.Close() | |
| if resp.StatusCode != http.StatusOK { | |
| fmt.Printf("API returned status code: %d\n", resp.StatusCode) | |
| return | |
| } | |
| fmt.Println("Entered normal mode successfully.") | |
| } | |
| func enterNormalMode() { | |
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | |
| defer cancel() | |
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, backendApiLocation+"/startNormalMode", nil) | |
| if err != nil { | |
| fmt.Printf("Failed to build request: %v\n", err) | |
| return | |
| } | |
| resp, err := http.DefaultClient.Do(req) | |
| if err != nil { | |
| fmt.Printf("Failed to call API: %v\n", err) | |
| return | |
| } | |
| defer resp.Body.Close() | |
| if resp.StatusCode != http.StatusOK { | |
| fmt.Printf("API returned status code: %d\n", resp.StatusCode) | |
| return | |
| } | |
| fmt.Println("Entered normal mode successfully.") | |
| } |
🤖 Prompt for AI Agents
In frontend/main.go around lines 453 to 467, the HTTP call uses http.Get without
a context or timeout and doesn't handle request construction errors; replace it
with creating a context with a timeout (e.g., context.WithTimeout), build the
request with http.NewRequestWithContext and check its error, use an http.Client
(or the context) to perform the request, handle client.Do errors, and ensure
resp.Body is closed; also add required imports (context, time, and net/http if
not present) so the request will timeout instead of hanging when the backend is
unreachable.
1 participant
This pull request introduces several improvements to the USB communication and configuration handling in the backend, adds a new configuration generator tool, and updates device management for increased reliability and usability. The main changes focus on making USB operations thread-safe, refactoring the pin configuration writing logic, and providing a web-based config generator.
USB Communication and Device Management
usbMutex) to synchronize USB operations, ensuring thread-safe access to USB devices in bothESP32MidiListenerandWriteToUSBmethods. Improved logging and stop signal handling for the ESP32 MIDI listener. [1] [2] [3] [4] [5]Configuration Handling
WritePinConfigto send a custom string format instead of JSON, and removed the call to stop the ESP32 MIDI listener before writing. [1] [2]HTTP API and Routing
/startNormalModethat starts the ESP32 MIDI listener in normal mode, allowing runtime control via the API. [1] [2]main.goto delegate control to the new API route.Configuration Generator Tool
configMaker/main.gothat provides a web interface to generate pin configuration JSON for multiplexers.Device List Update
usb_ports.jsonto reflect new device names and paths, expanding the available device list for selection.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores