Skip to content

refactor - #1

Open
sgardoll wants to merge 1 commit into
mainfrom
beta
Open

refactor#1
sgardoll wants to merge 1 commit into
mainfrom
beta

Conversation

@sgardoll

Copy link
Copy Markdown
Owner

@sgardoll

Copy link
Copy Markdown
Owner Author

@greptileai

1 similar comment
@sgardoll

Copy link
Copy Markdown
Owner Author

@greptileai

@greptile-apps

greptile-apps Bot commented Dec 18, 2025

Copy link
Copy Markdown

Greptile Summary

Refactored both main.py and server.js to improve code organization and maintainability. Key changes include:

main.py improvements:

  • Extracted magic numbers into constants (POLL_INTERVAL, NETWORK_TIMEOUT)
  • Removed global current_state variable in favor of local scope
  • Optimized garbage collection to run only on state changes
  • Simplified boolean checks and increased main loop sleep interval

server.js improvements:

  • Consolidated configuration into CONFIG object
  • Centralized all state into single state object
  • Added environment variable validation
  • Added SoX availability check on startup
  • Implemented graceful shutdown handlers (SIGINT/SIGTERM)
  • Moved helper functions before usage for better readability
  • Added recording error handler
  • Improved WebSocket state checks before sending

Critical issues found:

  • main.py: Infinite WiFi retry loop will hang device if credentials are incorrect
  • server.js: HTTP response sent inside async WebSocket 'open' handler causes request to hang
  • server.js: Missing response on audio initialization errors
  • server.js: Race condition in /stop endpoint returning stale session status
  • server.js: Missing error handler for speaker stream

Confidence Score: 2/5

  • This PR has critical async bugs that will cause production failures
  • Score reflects critical logical errors in server.js that cause HTTP request hangs and race conditions, plus an infinite loop in main.py that can brick the device. The refactoring improves code structure, but introduces bugs that will prevent the system from working reliably.
  • server.js requires immediate attention to fix async response handling in the /start endpoint. main.py needs WiFi retry limits to prevent infinite loops.

Important Files Changed

Filename Overview
main.py Refactored constants and state management, improved code clarity, but introduced infinite WiFi retry loop that can cause device to hang indefinitely
server.js Major refactor with better organization and error handling, but has critical async bugs causing request hangs and race conditions in WebSocket response handling

Sequence Diagram

sequenceDiagram
    participant Presto as Presto Device
    participant Server as Node.js Server
    participant ElevenLabs as ElevenLabs API
    participant Mic as Microphone
    participant Speaker as Speaker

    Note over Presto: Boot & WiFi Connect
    Presto->>Server: GET /status
    Server-->>Presto: {sessionState: "idle"}
    Note over Presto: Display IDLE

    Note over Presto: User Taps Screen
    Presto->>Server: POST /start
    
    Server->>ElevenLabs: WebSocket Connect
    ElevenLabs-->>Server: Connection Open
    
    Server->>Mic: Start Recording (SoX)
    Server->>Speaker: Initialize Speaker
    Server-->>Presto: {ok: true, sessionState: "started"}
    
    Note over Presto: Display ACTIVE

    loop Audio Processing
        Mic->>Server: Audio Chunk (PCM)
        Server->>Server: Calculate RMS (Volume)
        alt Volume > Threshold
            Server->>ElevenLabs: Send Audio (Base64)
        else Volume <= Threshold
            Note over Server: Drop (Echo/Silence)
        end
        
        ElevenLabs->>Server: Audio Response (Base64)
        Server->>Speaker: Play Audio
        Note over Server: Set agentIsSpeaking=true
    end

    Note over Presto: User Taps Again
    Presto->>Server: POST /stop
    Server->>ElevenLabs: Close WebSocket
    Server->>Mic: Stop Recording
    Server->>Speaker: End Playback
    Server-->>Presto: {ok: true, sessionState: "idle"}
    
    Note over Presto: Display IDLE
Loading

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

Comment thread main.py
Comment on lines +111 to +113
while not connect_wifi():
draw_ui("offline", "WiFi Failed. Retrying...")
time.sleep(3)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: infinite loop with no escape mechanism - if WiFi credentials are wrong or router is down, device will be stuck forever with no way to recover without physical reset

Suggested change
while not connect_wifi():
draw_ui("offline", "WiFi Failed. Retrying...")
time.sleep(3)
# 2. Connect with Retry (max attempts)
retry_count = 0
max_retries = 5
while not connect_wifi() and retry_count < max_retries:
retry_count += 1
draw_ui("offline", f"WiFi Failed. Retry {retry_count}/{max_retries}...")
time.sleep(3)
if retry_count >= max_retries:
draw_ui("offline", "WiFi Failed. Check credentials.")
return
Prompt To Fix With AI
This is a comment left during a code review.
Path: main.py
Line: 111:113

Comment:
**logic:** infinite loop with no escape mechanism - if WiFi credentials are wrong or router is down, device will be stuck forever with no way to recover without physical reset

```suggestion
    # 2. Connect with Retry (max attempts)
    retry_count = 0
    max_retries = 5
    while not connect_wifi() and retry_count < max_retries:
        retry_count += 1
        draw_ui("offline", f"WiFi Failed. Retry {retry_count}/{max_retries}...")
        time.sleep(3)
    
    if retry_count >= max_retries:
        draw_ui("offline", "WiFi Failed. Check credentials.")
        return
```

How can I resolve this? If you propose a fix, please make it concise.

Comment thread server.js
cleanup();
});

res.json({ ok: true, sessionState: "started" });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: response sent inside async 'open' event handler - the HTTP request will hang because res.json() is called after WebSocket opens, but the /start route returns immediately without waiting. Is the client expecting an immediate response or should it wait for WebSocket connection?

Prompt To Fix With AI
This is a comment left during a code review.
Path: server.js
Line: 158:158

Comment:
**logic:** response sent inside async 'open' event handler - the HTTP request will hang because `res.json()` is called after WebSocket opens, but the `/start` route returns immediately without waiting. Is the client expecting an immediate response or should it wait for WebSocket connection?

How can I resolve this? If you propose a fix, please make it concise.

Comment thread server.js
Comment on lines +160 to +169
} catch (audioErr) {
console.error("Audio initialization failed:", audioErr);
cleanup();
// Since we already responded (maybe), we can't easily res.json if we were inside an async callback,
// but here we are inside the sync 'open' handler, so the response hasn't been sent yet?
// Actually 'open' is async relative to the request. The request is pending.
// Wait, 'res.json' is called at the end of 'open'.
// We should ensure we don't double respond.
// In this flow, we haven't responded yet.
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: error handling missing response - if audio initialization fails inside the 'open' handler, cleanup() is called but no response is sent to the client, causing the request to hang indefinitely

Prompt To Fix With AI
This is a comment left during a code review.
Path: server.js
Line: 160:169

Comment:
**logic:** error handling missing response - if audio initialization fails inside the 'open' handler, `cleanup()` is called but no response is sent to the client, causing the request to hang indefinitely

How can I resolve this? If you propose a fix, please make it concise.

Comment thread server.js
cleanup();
sessionState = "idle";
res.json({ ok: true, sessionState });
res.json({ ok: true, sessionState: state.sessionStatus });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: race condition - sessionStatus set to "idle" inside cleanup() (line 71) which runs asynchronously when WebSocket closes, but response returns state.sessionStatus immediately, potentially returning stale "listening" or "error" status

Prompt To Fix With AI
This is a comment left during a code review.
Path: server.js
Line: 215:215

Comment:
**logic:** race condition - `sessionStatus` set to "idle" inside `cleanup()` (line 71) which runs asynchronously when WebSocket closes, but response returns `state.sessionStatus` immediately, potentially returning stale "listening" or "error" status

How can I resolve this? If you propose a fix, please make it concise.

Comment thread server.js
Comment on lines +153 to +156
state.recording.stream().on("error", (err) => {
console.error("Recording error:", err);
cleanup();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: no error handler for speaker stream - if speaker fails (e.g., audio device disconnected), the error will crash the process since there's no .on('error') handler attached to the speaker stream

Prompt To Fix With AI
This is a comment left during a code review.
Path: server.js
Line: 153:156

Comment:
**logic:** no error handler for speaker stream - if speaker fails (e.g., audio device disconnected), the error will crash the process since there's no `.on('error')` handler attached to the speaker stream

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant