feat(moneo): recorder + main sketch (single-file WAV, reliable touch)#6
feat(moneo): recorder + main sketch (single-file WAV, reliable touch)#6Ricky1207-sen wants to merge 3 commits into
Conversation
debloper
left a comment
There was a problem hiding this comment.
Some changes are necessary, some are good to have and rest of them are comments/FYI.
| const int TOUCH_THRESHOLD = 22000; | ||
| const unsigned long DEBOUNCE_DELAY = 2000; | ||
| const int TOUCH_THRESHOLD = 50000; | ||
| const unsigned long DEBOUNCE_DELAY = 5000; |
There was a problem hiding this comment.
That's too long IMO; but not a blocker.
| void setup() { | ||
| Serial.begin(115200); | ||
| unsigned long t = millis(); | ||
| while (!Serial && millis() - t < 500) delay(10); |
There was a problem hiding this comment.
By the time you're counting the millis() it may already be over 500ms (as in, it's not a deterministic timeout).
A better way could be:
unsigned long serialTimeout = millis() + 500;
while (!Serial && millis() < serialTimeout) { delay(10); }
In this way the timeout starts when we start waiting for Serial, and waits for 500ms (however, I think 100ms is more than enough). Also, probably should put the waiting duration inside the Config.h instead of putting the raw values here.
|
|
||
| touchAttachInterrupt(TOUCH_PIN, onTouch, TOUCH_THRESHOLD); | ||
|
|
||
| Serial.println("[Moneo] Ready. Touch pin D1 to start recording."); |
There was a problem hiding this comment.
Redundant consecutive instructions to touch pin to start recording (recorder.begin already logs this just above). One of those is not necessary.
| } | ||
| _wasRecording = nowRecording; | ||
|
|
||
| delay(10); |
There was a problem hiding this comment.
Too short interval I think... checking each 0.1sec (100ms) is more than sufficient. Even 1s/1000ms would be snappy enough for any practical purposes.
| {} | ||
|
|
||
| bool Recorder::begin() { | ||
| if (!SD.begin(SD_CARD_PIN)) { |
There was a problem hiding this comment.
Would work as SD.h etc. are included in the Recorder.h file, but the best practice is to include the headers where they're being used... so in this file.
Not a blocker/must.
| DLOG("[Recorder] I2S init failed!"); | ||
| return false; | ||
| } | ||
| DLOG("[Recorder] I2S initialized (16kHz, 8-bit)."); |
There was a problem hiding this comment.
Don't hardcode config parameters in log.
It can be misleading while debugging (if the config values changed, and then forgotten).
| void Recorder::requestToggle() { | ||
| _toggleRequested = true; | ||
| } |
There was a problem hiding this comment.
The nested wrapping of onTouch -> requestToggle -> toggleRequested seems a bit too deep fried.
It will work, so not a blocker... but perhaps not the most elegant way.
| _psramWritten = 0; | ||
|
|
||
| // Try NTP time sync | ||
| configTime(19800, 0, "pool.ntp.org"); // UTC+5:30 for IST |
There was a problem hiding this comment.
NTP sync should be in main INO file's setup section (as with any runtime blocking/stalling subroutine), and kept alongside the lastTimeSync as millis() value when it was sync'd, so that current time can always be calculated without needing to make another network request (by adding the elapsed time sinch the last sync, with the last sync'd timestamp).
As it currently is, nothing is even ensuring the WiFi is connected (most likely isn't) so the request would just fail.
| xTaskCreatePinnedToCore(_captureTaskEntry, "Capture", 4096, | ||
| this, 5, &_captureTask_h, 1); | ||
| xTaskCreatePinnedToCore(_writerTaskEntry, "Writer", 8192, | ||
| this, 3, &_writerTask_h, 1); |
There was a problem hiding this comment.
This is nice to use the second core for compute heavy non-management task. Will have to check how it works out in terms of real world impact.
| File f = SD.open(_wavPath.c_str(), FILE_APPEND); | ||
| if (!f) { | ||
| // Keep the buffer and retry on the next cycle rather than dropping audio. | ||
| DLOG("[Writer] Append open failed; will retry next segment."); |
There was a problem hiding this comment.
NOPE... that's gonna be a corrupt/incomplete recording (with parts missing).
Log the failure and close the recording. Blink error.
There was a problem hiding this comment.
Pull request overview
Adds the Stage 2 moneo recording pipeline: a PSRAM-buffered recorder that appends audio into a single WAV on SD in periodic segments, plus a main sketch that drives touch-to-record and WiFi connect-on-stop behavior.
Changes:
- Introduces
Recorder(FreeRTOS capture/writer tasks) that writes one WAV per session with periodic append/close cycles. - Adds
moneo.inomain sketch wiring touch interrupt → recorder lifecycle → WiFi connection after recording completes. - Adjusts touch threshold and debounce constants in
Config.hfor improved touch stability.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| moneo/Recorder.h | Declares the recorder API and task/buffer state for PSRAM-based capture + periodic SD append. |
| moneo/Recorder.cpp | Implements WAV header writing, capture/writer tasks, segment flushing, and start/stop lifecycle. |
| moneo/moneo.ino | Main Arduino sketch wiring touch interrupt handling, recording state transitions, and WiFi processing on stop. |
| moneo/Config.h | Updates touch threshold/debounce values for noise rejection and stability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Confirm a real touch is still present. The interrupt can fire on brief | ||
| // electrical noise; a genuine press is still held when we reach here (reads | ||
| // above threshold), while a noise spike has already decayed and is rejected. | ||
| if (touchRead(TOUCH_PIN) < TOUCH_THRESHOLD) return; | ||
|
|
| bool begin(); | ||
| void loop(); | ||
| void requestToggle(); | ||
|
|
| void Recorder::requestToggle() { | ||
| _toggleRequested = true; | ||
| } |
| size_t written = f.write(_psramBuf, _psramWritten); | ||
| f.close(); | ||
|
|
||
| _dataLength += written; | ||
| _psramWritten = 0; | ||
| DLOGF("[Writer] Appended %u bytes (total: %u)\n", written, _dataLength); |
| // Append whatever is left in the buffer. The tasks have stopped, so no | ||
| // mutex is needed here. | ||
| _flushSegment(); |
| // Try NTP time sync | ||
| configTime(19800, 0, "pool.ntp.org"); // UTC+5:30 for IST | ||
| delay(500); | ||
|
|
||
| _wavPath = _generateFilename(); |
…cludes, log/serial cleanups
Adds the moneo recorder and the main sketch that drives it.
Recorder (Recorder.cpp/.h)
Touch reliability
(was causing phantom start/stop and auto-restarts).
Main sketch (moneo.ino)
in the repo yet — enabled in a follow-up PR.