From 6010751b17666ce79b178749e43caeca647dfa1d Mon Sep 17 00:00:00 2001 From: jordlee Date: Thu, 13 Aug 2026 13:33:16 -0700 Subject: [PATCH 1/6] fix(server): do not destroy a joinable server thread during shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stop() opened with `if (!m_running) return;` above the join, so the second caller — ~CameraWebServer(), after the detached POST /api/server/shutdown thread had already cleared the flag — returned early and skipped it. m_serverThread was then destroyed while still joinable: std::terminate, SIGABRT, exit 134 after an otherwise complete teardown. - Serialize stop() and stopLiveViewBroadcasting() on a new m_shutdownMutex and always join. A bare exchange() would still have left both callers able to join the same std::thread concurrently, which is itself undefined. - stopLiveViewBroadcasting() had the identical early-return-above-join shape and is fixed the same way. - The signal handler no longer calls stop(). It performed stream I/O, thread joins and mutex acquisition inside a signal context — already unsafe, and with the new mutex it could deadlock and wedge Ctrl-C. It now records the signal and main() performs the teardown. Verified on hardware (macOS, ILCE-7M5): SIGINT, SIGTERM, triple rapid SIGINT, SIGINT with a camera connected, and two HTTP shutdowns — all exit 0, exactly one teardown each, with ReleaseDevice() returning cleanly. Refs #42 Co-Authored-By: Claude Opus 5 --- api/server/src/CameraWebApp.cpp | 33 +++++++++++++----- api/server/src/CameraWebServer.cpp | 55 +++++++++++++++++++++--------- api/server/src/CameraWebServer.h | 4 +++ 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/api/server/src/CameraWebApp.cpp b/api/server/src/CameraWebApp.cpp index b5cbedd..42d942b 100644 --- a/api/server/src/CameraWebApp.cpp +++ b/api/server/src/CameraWebApp.cpp @@ -10,12 +10,17 @@ // Global server instance for signal handling (atomic for safe concurrent access) std::atomic g_server{nullptr}; +// Set by signalHandler, consumed by main()'s run loop. +std::atomic g_signalReceived{0}; + +// Async-signal-safe: records the signal and returns. It must NOT call stop() +// directly — stop() does stream I/O, joins threads, and takes several mutexes, +// none of which are legal in a signal handler. Doing so also risks a hard +// deadlock: a signal delivered to a thread already holding the shutdown mutex +// would block the handler forever and make Ctrl-C stop working. main() does the +// actual teardown once it observes this flag. void signalHandler(int signal) { - std::cout << "\nReceived signal " << signal << ", shutting down web server..." << std::endl; - auto* server = g_server.load(); - if (server) { - server->stop(); - } + g_signalReceived.store(signal); } int main(int argc, char* argv[]) { @@ -72,11 +77,23 @@ int main(int argc, char* argv[]) { std::cout << "Press Ctrl+C to stop the server..." << std::endl; std::cout << std::endl; - // Keep server running until interrupted - while (server.isRunning()) { - std::this_thread::sleep_for(std::chrono::seconds(1)); + // Keep server running until interrupted, or until something else (the + // detached POST /api/server/shutdown thread) clears the running flag. + while (server.isRunning() && g_signalReceived.load() == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(200)); } + if (const int sig = g_signalReceived.load()) { + std::cout << "\nReceived signal " << sig << ", shutting down web server..." << std::endl; + } + + // Tear down explicitly rather than leaving it to ~CameraWebServer(). stop() + // is idempotent and serialized, so this is safe even when the detached + // shutdown thread is already inside it — we simply block until it is done. + // Doing it here (not in the destructor) keeps the join on a thread that is + // still fully constructed. + server.stop(); + // Null out global pointer before server goes out of scope (prevents signal handler dangling pointer) g_server.store(nullptr); diff --git a/api/server/src/CameraWebServer.cpp b/api/server/src/CameraWebServer.cpp index 398a742..f4339f3 100644 --- a/api/server/src/CameraWebServer.cpp +++ b/api/server/src/CameraWebServer.cpp @@ -168,20 +168,40 @@ bool CameraWebServer::start() { } void CameraWebServer::stop() { - if (!m_running) return; - - std::cout << "[Shutdown] Stopping web server..." << std::endl; - m_running = false; - - if (m_serverSocket >= 0) { - close(m_serverSocket); - m_serverSocket = -1; - } - + // Serialized, and deliberately WITHOUT an early return before the join. + // + // stop() is reached from three directions: the detached thread spawned by + // POST /api/server/shutdown, main()'s run loop, and ~CameraWebServer(). + // The old `if (!m_running) return;` guard sat above the join, so the second + // caller returned early and skipped it — leaving m_serverThread joinable at + // destruction, which is std::terminate → SIGABRT → exit 134. + // + // The mutex matters as much as the missing join: without it the two callers + // could reach m_serverThread.join() concurrently, and joining the same + // thread from two threads is itself undefined. Holding it across the whole + // teardown also stops ~CameraWebServer() from destroying members while the + // detached thread is still inside here. + std::lock_guard guard(m_shutdownMutex); + const bool wasRunning = m_running.exchange(false); + + if (wasRunning) { + std::cout << "[Shutdown] Stopping web server..." << std::endl; + + if (m_serverSocket >= 0) { + close(m_serverSocket); + m_serverSocket = -1; + } + } + + // Always join. After the first caller joins, the thread is no longer + // joinable, so later callers fall through harmlessly. if (m_serverThread.joinable()) { m_serverThread.join(); } + // The remainder is one-shot teardown; the first caller already ran it. + if (!wasRunning) return; + // Close all SSE client sockets to unblock their keepalive loops { std::lock_guard lock(m_sseClientsMutex); @@ -1706,15 +1726,18 @@ void CameraWebServer::startLiveViewBroadcasting() { } void CameraWebServer::stopLiveViewBroadcasting() { - if (!m_broadcastingLiveView.load()) { - return; - } - - m_broadcastingLiveView = false; + // Same shape as stop(), and the same hazard: the early return sat above the + // join, so a second caller could leave m_broadcastThread joinable and abort + // at destruction. Shares stop()'s mutex so the destructor's two teardown + // calls cannot interleave with the detached shutdown thread. Neither + // function calls the other, so there is no lock ordering to get wrong. + std::lock_guard guard(m_shutdownMutex); + m_broadcastingLiveView.exchange(false); + if (m_broadcastThread.joinable()) { m_broadcastThread.join(); + std::cout << "Stopped live view WebSocket broadcasting" << std::endl; } - std::cout << "Stopped live view WebSocket broadcasting" << std::endl; } void CameraWebServer::liveViewBroadcastThread() { diff --git a/api/server/src/CameraWebServer.h b/api/server/src/CameraWebServer.h index c654e03..34c9912 100644 --- a/api/server/src/CameraWebServer.h +++ b/api/server/src/CameraWebServer.h @@ -80,6 +80,10 @@ class CameraWebServer { int m_serverSocket; std::atomic m_running; std::thread m_serverThread; + // Serializes stop() / stopLiveViewBroadcasting(), which are each reachable + // from the detached shutdown thread, main(), and the destructor. Guards the + // joins so no two callers can join the same std::thread concurrently. + std::mutex m_shutdownMutex; std::unique_ptr m_cameraController; // WebSocket support From 9152cd81135461d83a371bc592dc0106756d3ed1 Mon Sep 17 00:00:00 2001 From: jordlee Date: Thu, 13 Aug 2026 13:33:16 -0700 Subject: [PATCH 2/6] fix(server): honour the documented lines and level params on /api/server/logs The handler hardcoded maxLines = 100 and minLevel = "info", with a comment saying the values "could be parsed from request.path". They could not: parseRequest() stripped the query string off the path and discarded it, so no handler could ever observe a query parameter. - Retain the query string on HttpRequest (routing still matches the bare path) and add HttpRequest::queryParam(). - Parse ?lines= (clamped to the retained buffer; non-numeric falls back to the default) and ?level= (unrecognised falls back to info). - Report `returned` alongside `total`, so a filtered response is not mistaken for data loss. Verified: ?lines=5 returns 5, ?level=warn returns warn entries only, ?lines=3&level=warn combines both, and ?lines=notanumber / ?level=bogus fall back gracefully. Routing was re-checked across 11 endpoints with and without query strings. Refs #46 Co-Authored-By: Claude Opus 5 --- api/server/src/CameraWebServer.cpp | 55 ++++++++++++++++++++++-------- api/server/src/CameraWebServer.h | 8 ++++- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/api/server/src/CameraWebServer.cpp b/api/server/src/CameraWebServer.cpp index f4339f3..5f2abcc 100644 --- a/api/server/src/CameraWebServer.cpp +++ b/api/server/src/CameraWebServer.cpp @@ -334,21 +334,40 @@ void CameraWebServer::handleClient(int clientSocket) { close(clientSocket); } +std::string HttpRequest::queryParam(const std::string& name, const std::string& fallback) const { + size_t pos = 0; + while (pos < query.size()) { + const size_t amp = query.find('&', pos); + const size_t end = (amp == std::string::npos) ? query.size() : amp; + const size_t eq = query.find('=', pos); + if (eq != std::string::npos && eq < end && query.compare(pos, eq - pos, name) == 0) { + std::string value = query.substr(eq + 1, end - eq - 1); + if (!value.empty()) return value; + } + if (amp == std::string::npos) break; + pos = amp + 1; + } + return fallback; +} + HttpRequest CameraWebServer::parseRequest(const std::string& request) { HttpRequest req; std::istringstream iss(request); std::string line; - + // Parse request line (GET /path HTTP/1.1) if (std::getline(iss, line)) { std::istringstream requestLine(line); std::string httpVersion; requestLine >> req.method >> req.path >> httpVersion; - // Remove query parameters if present + // Split the query string off the path. Routing matches on the bare + // path, but the query is kept — it used to be discarded here, which is + // why handlers documenting `?lines=`/`?level=` could never honour them. size_t queryPos = req.path.find('?'); if (queryPos != std::string::npos) { + req.query = req.path.substr(queryPos + 1); req.path = req.path.substr(0, queryPos); } } @@ -3282,24 +3301,30 @@ HttpResponse CameraWebServer::handleApiServerLogs(const HttpRequest& request) { response.contentType = "application/json"; response.statusCode = 200; - // Parse query parameters: ?lines=100&level=info - int maxLines = 100; - std::string minLevel = "info"; - - // Query params could be parsed from request.path if needed in the future - - std::lock_guard lock(m_logMutex); - - // Filter by level auto levelPriority = [](const std::string& level) -> int { if (level == "debug") return 0; if (level == "info") return 1; if (level == "warn") return 2; if (level == "error") return 3; - return 1; + return -1; // unrecognised }; - int minPriority = levelPriority(minLevel); + // Parse query parameters: ?lines=100&level=info + int maxLines = 100; + try { + const int requested = std::stoi(request.queryParam("lines", "100")); + // Clamp rather than reject: a caller asking for more than we retain + // should get everything we have, not an error. + maxLines = std::max(1, std::min(requested, static_cast(MAX_LOG_ENTRIES))); + } catch (const std::exception&) { + maxLines = 100; // non-numeric ?lines= falls back to the default + } + + const std::string requestedLevel = request.queryParam("level", "info"); + int minPriority = levelPriority(requestedLevel); + if (minPriority < 0) minPriority = levelPriority("info"); + + std::lock_guard lock(m_logMutex); std::ostringstream json; json << "{\n \"success\": true,\n \"logs\": [\n"; @@ -3323,7 +3348,9 @@ HttpResponse CameraWebServer::handleApiServerLogs(const HttpRequest& request) { } } - json << "\n ],\n \"total\": " << total << "\n}"; + // `total` is everything retained; `returned` is what survived the lines/level + // filter. Without both, a filtered response looks like data loss. + json << "\n ],\n \"returned\": " << count << ",\n \"total\": " << total << "\n}"; response.body = json.str(); return response; diff --git a/api/server/src/CameraWebServer.h b/api/server/src/CameraWebServer.h index 34c9912..8c396d9 100644 --- a/api/server/src/CameraWebServer.h +++ b/api/server/src/CameraWebServer.h @@ -33,10 +33,16 @@ namespace cli { struct HttpRequest { std::string method; - std::string path; + std::string path; // query string stripped, so routing stays exact-match + std::string query; // raw query string, without the leading '?' std::string body; std::string headers; std::string contentType; + + // Value of `name` from the query string, or `fallback` if absent/empty. + // Percent-decoding is deliberately not attempted: the only consumers today + // are numeric/enum scalars, and a half-correct decoder is worse than none. + std::string queryParam(const std::string& name, const std::string& fallback = "") const; }; struct HttpResponse { From 3d96992e212f6c7cac2d61360914deb362b4a82d Mon Sep 17 00:00:00 2001 From: jordlee Date: Thu, 13 Aug 2026 13:33:16 -0700 Subject: [PATCH 3/6] fix(server): report 0x8D03 as a retryable 409 rather than a hard 400 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CrError_RemoteTransfer_GetContentsDataDisable does not mean the requested transfer failed — it means one is already in flight, and it clears as soon as that transfer completes. Reporting it as 400 "Failed to start file download" is what led #40 to conclude that a single stuck transfer permanently "latches" the RemoteTransfer subsystem; in fact every later request was correctly reporting "busy" because the stuck one never finished. - Add classifyTransferError(), shared by the full-file and thumbnail/screennail paths so the two cannot drift. - Map 0x8D03 to 409 with a message naming the state and the remedy, plus retryable: true. 0x8D02 keeps its existing 400 and message. - Log the outcome at warn/error. These paths previously logged nothing beyond the request line, which is a large part of why #40 was hard to diagnose. Verified: a second request issued 0.3s into a 67MB transfer returns 409, and the identical request returns 202 once that transfer completes. Refs #49, refs #40 Co-Authored-By: Claude Opus 5 --- api/server/src/CameraWebServer.cpp | 55 +++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/api/server/src/CameraWebServer.cpp b/api/server/src/CameraWebServer.cpp index 5f2abcc..6965327 100644 --- a/api/server/src/CameraWebServer.cpp +++ b/api/server/src/CameraWebServer.cpp @@ -2553,6 +2553,42 @@ HttpResponse CameraWebServer::handleApiListSDCardFiles(const std::string& camera return response; } +namespace { + +// Maps a failed RemoteTransfer start into an HTTP status + a message that says +// what actually happened. Shared by the full-file and thumbnail/screennail +// paths so the two cannot drift. +// +// 0x8D03 (CrError_RemoteTransfer_GetContentsDataDisable) is the important one: +// it does NOT mean the requested transfer failed, it means one is already in +// flight. It clears on its own the moment the in-flight transfer finishes, so +// it is a 409 the caller should retry — not a 400. Reporting it as a hard +// failure is what led the investigation in #40 to conclude that a single stuck +// transfer "latches" the subsystem; in fact every later request was correctly +// reporting "busy" because the stuck one never completed. +struct TransferError { + int statusCode; + std::string message; +}; + +TransferError classifyTransferError(const std::string& sdkMessage, const std::string& what) { + if (sdkMessage.find("0x00008D03") != std::string::npos) { + return {409, + "A transfer is already in progress on this camera, so the " + what + + " was not started. This is transient — retry once the current transfer " + "completes (watch for a transferProgress event). SDK error 0x00008D03."}; + } + if (sdkMessage.find("0x00008D02") != std::string::npos) { + return {400, + "Failed to start " + what + + ". Confirm the file identifiers are valid for the current connection " + "mode and retry (SDK error 0x00008D02)."}; + } + return {400, sdkMessage}; +} + +} // namespace + HttpResponse CameraWebServer::handleApiDownloadSDCardFile( const std::string& cameraId, const std::string& slotNumber, @@ -2616,12 +2652,11 @@ HttpResponse CameraWebServer::handleApiDownloadSDCardFile( root["message"] = result.message.empty() ? "Download started" : result.message; response.statusCode = 202; // Accepted — download is async } else { - std::string message = result.error_message; - if (message.find("0x00008D02") != std::string::npos) { - message = "Failed to start file download. Confirm the file identifiers are valid for the current connection mode and retry (SDK error 0x00008D02)."; - } - root["message"] = message; - response.statusCode = 400; + const auto classified = classifyTransferError(result.error_message, "file download"); + root["message"] = classified.message; + root["retryable"] = (classified.statusCode == 409); + response.statusCode = classified.statusCode; + addLog(classified.statusCode == 409 ? "warn" : "error", classified.message, cameraId); } response.body = root.toStyledString(); @@ -2686,8 +2721,12 @@ HttpResponse CameraWebServer::handleApiDownloadCompressed( root["type"] = type; response.statusCode = 202; } else { - root["message"] = result.error_message; - response.statusCode = 400; + const auto classified = classifyTransferError(result.error_message, type + " download"); + root["message"] = classified.message; + root["type"] = type; + root["retryable"] = (classified.statusCode == 409); + response.statusCode = classified.statusCode; + addLog(classified.statusCode == 409 ? "warn" : "error", classified.message, cameraId); } response.body = root.toStyledString(); From 1cd65d955ed46c1399968fd3ef464d5c9bd3fc02 Mon Sep 17 00:00:00 2001 From: jordlee Date: Thu, 13 Aug 2026 13:33:16 -0700 Subject: [PATCH 4/6] fix(server): emit the documented identifiers on transferProgress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The event carried only {percent, notify, filename} — none of the cameraId, contentId, fileId or savedPath the spec documents — so a client written against the spec could not tell which transfer had completed. - Track the in-flight transfer's content and file id, and emit them along with cameraId and savedPath, from both the SDK callback and the polling fallback. - Hold that identity in dedicated members rather than reading it from m_pendingTransfers. Once a real callback has been seen, transferPollLoop() clears that list on every tick, so it is already empty by the time the callback for any transfer after the first arrives; reading it there reported contentId 0 for every transfer but the first. - Keep `filename` alongside the new `savedPath`. mcp/src/tools/files.ts reads it, and removing it would break the MCP client. - Correct the event catalog: drop the stale claim that the macOS SDK callback is unreliable — it is not on V2.02, where no synthetic events were observed — and document notify, filename and synthetic. Verified: three sequential transfers each emitted the correct distinct contentId with every documented field present, and the MCP client still resolves the saved path end to end. Refs #47 Co-Authored-By: Claude Opus 5 --- api/openapi.yaml | 2 +- api/server/src/device/CameraDeviceRest.cpp | 48 ++++++++++++++++++---- api/server/src/device/CameraDeviceRest.h | 15 +++++++ 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/api/openapi.yaml b/api/openapi.yaml index 2137fb5..73ab313 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -1301,7 +1301,7 @@ paths: | `propertyChanged` | `OnPropertyChangedCodes` | `{cameraId, codes: [...]}` | One or more property values changed on the camera | | `afStatus` | `OnWarning` (`CrWarningExt_AFStatus`) | `{state: "focused" \| "unlocked" \| "tracking", source?}` | Autofocus state changed | | `downloadComplete` | `OnCompleteDownload` | `{cameraId, filename, savedPath}` | Auto-transferred image saved to host (remote mode + matching `still-image-store-destination`) | - | `transferProgress` | `OnNotifyRemoteTransferResult` (or filesystem polling fallback) | `{cameraId, contentId, fileId, percent, savedPath?}` | Explicit file-pull progress (remote-transfer mode). On macOS V2.01 the SDK callback is unreliable; the server falls back to filesystem polling and emits synthetic events | + | `transferProgress` | `OnNotifyRemoteTransferResult` (or filesystem polling fallback) | `{cameraId, contentId, fileId, percent, notify, savedPath?, filename?, synthetic?}` | Explicit file-pull progress (remote-transfer mode). `filename` duplicates `savedPath` and is retained for existing consumers — prefer `savedPath`. `synthetic: true` marks an event produced by the filesystem-polling fallback rather than the SDK callback; on SDK V2.02 (macOS/Linux) the real callback fires reliably and `synthetic` is absent | | `contentsTransfer` | `OnContentsTransfer` | `{cameraId, files: [...]}` | Contents-transfer mode file list update (printable-ASCII filenames only) | | `operationResult` | `OnCompleteOperation` | `{cameraId, operation, result}` | Generic SDK operation completed (e.g. control codes, non-shutter commands) | | `error` | `OnError` | `{cameraId, code, message}` | SDK error occurred | diff --git a/api/server/src/device/CameraDeviceRest.cpp b/api/server/src/device/CameraDeviceRest.cpp index 5e54aa9..7ae15fa 100644 --- a/api/server/src/device/CameraDeviceRest.cpp +++ b/api/server/src/device/CameraDeviceRest.cpp @@ -547,7 +547,12 @@ CameraDeviceRest::FileDownloadResult CameraDeviceRest::download_remote_transfer_ { std::lock_guard lock(m_pendingTransfersMtx); m_pendingTransfers.push_back( - {effectiveSaveDir, std::move(preSnapshot), std::chrono::steady_clock::now()}); + {effectiveSaveDir, std::move(preSnapshot), std::chrono::steady_clock::now(), + static_cast(content_id), static_cast(file_id)}); + } + m_inFlightContentId.store(static_cast(content_id)); + m_inFlightFileId.store(static_cast(file_id)); + { } startTransferPolling(); @@ -592,7 +597,12 @@ CameraDeviceRest::FileDownloadResult CameraDeviceRest::download_remote_transfer_ { std::lock_guard lock(m_pendingTransfersMtx); m_pendingTransfers.push_back( - {effectiveSaveDir, std::move(preSnapshot), std::chrono::steady_clock::now()}); + {effectiveSaveDir, std::move(preSnapshot), std::chrono::steady_clock::now(), + static_cast(content_id), static_cast(file_id)}); + } + m_inFlightContentId.store(static_cast(content_id)); + m_inFlightFileId.store(static_cast(file_id)); + { } startTransferPolling(); @@ -633,7 +643,12 @@ CameraDeviceRest::FileDownloadResult CameraDeviceRest::download_remote_transfer_ { std::lock_guard lock(m_pendingTransfersMtx); m_pendingTransfers.push_back( - {effectiveSaveDir, std::move(preSnapshot), std::chrono::steady_clock::now()}); + {effectiveSaveDir, std::move(preSnapshot), std::chrono::steady_clock::now(), + static_cast(content_id), static_cast(file_id)}); + } + m_inFlightContentId.store(static_cast(content_id)); + m_inFlightFileId.store(static_cast(file_id)); + { } startTransferPolling(); @@ -1463,17 +1478,28 @@ void CameraDeviceRest::OnNotifyRemoteTransferResult(CrInt32u notify, CrInt32u pe // The SDK reports progress itself on this build, so the disk-polling // fallback must stay out of the way from now on. m_realTransferCallbackSeen.store(true); - // A real SDK callback fired — cancel the disk-polling fallback. + // A real SDK callback fired — cancel the disk-polling fallback. Capture the + // pending entry's identifiers first: the SDK hands us only a filename, and + // the spec's transferProgress carries contentId/fileId. + const unsigned int contentId = m_inFlightContentId.load(); + const unsigned int fileId = m_inFlightFileId.load(); { std::lock_guard lock(m_pendingTransfersMtx); m_pendingTransfers.clear(); } if (m_eventCallback) { std::ostringstream oss; - oss << "{\"percent\":" << per << ",\"notify\":\"0x" << std::hex << notify << std::dec << "\""; + oss << "{\"percent\":" << per << ",\"notify\":\"0x" << std::hex << notify << std::dec << "\"" + << ",\"cameraId\":\"" << jsonEscape(std::string(get_id().data())) << "\"" + << ",\"contentId\":" << contentId + << ",\"fileId\":" << fileId; if (filename) { cli::text file(filename); - oss << ",\"filename\":\"" << jsonEscape(std::string(file.data())) << "\""; + const std::string path = jsonEscape(std::string(file.data())); + // `savedPath` is the documented field; `filename` is retained + // because existing consumers (mcp/src/tools/files.ts) read it. + oss << ",\"savedPath\":\"" << path << "\"" + << ",\"filename\":\"" << path << "\""; } oss << "}"; m_eventCallback("transferProgress", oss.str()); @@ -1551,8 +1577,14 @@ void CameraDeviceRest::transferPollLoop() { if (m_eventCallback) { std::ostringstream oss; // path.string() is a native host path — C:\Users\... on Windows. - oss << "{\"percent\":100,\"notify\":\"0x20093\",\"filename\":\"" - << jsonEscape(path.string()) << "\",\"synthetic\":true}"; + const std::string saved = jsonEscape(path.string()); + oss << "{\"percent\":100,\"notify\":\"0x20093\"" + << ",\"cameraId\":\"" << jsonEscape(std::string(get_id().data())) << "\"" + << ",\"contentId\":" << it->contentId + << ",\"fileId\":" << it->fileId + << ",\"savedPath\":\"" << saved << "\"" + << ",\"filename\":\"" << saved << "\"" + << ",\"synthetic\":true}"; m_eventCallback("transferProgress", oss.str()); } it = m_pendingTransfers.erase(it); diff --git a/api/server/src/device/CameraDeviceRest.h b/api/server/src/device/CameraDeviceRest.h index 300007a..1266ed5 100644 --- a/api/server/src/device/CameraDeviceRest.h +++ b/api/server/src/device/CameraDeviceRest.h @@ -56,6 +56,12 @@ struct PendingTransfer { std::string saveDir; // directory to poll std::set preSnapshot; // files present before download std::chrono::steady_clock::time_point startTime; + // Carried so completion events can identify *which* transfer finished — the + // SDK callback reports only a filename. The SDK permits one transfer at a + // time (a second start returns 0x8D03), so at most one entry is ever + // pending and the correlation is unambiguous. + unsigned int contentId = 0; + unsigned int fileId = 0; // The SDK creates the destination file when the transfer STARTS and streams // into it, so "a new file appeared" does not mean "the transfer finished". // Completion is only declared once the size stops changing between polls. @@ -378,6 +384,15 @@ class CameraDeviceRest : public SCRSDK::IDeviceCallback { // has fired we must never emit synthetic completions again, or clients get // a premature percent:100 while the file is still being written. std::atomic m_realTransferCallbackSeen{false}; + // Identity of the transfer currently in flight, for completion events. + // Deliberately NOT read from m_pendingTransfers: once a real SDK callback + // has been seen, transferPollLoop() clears that list on every tick to keep + // the disk-polling fallback out of the way, so it is empty by the time the + // callback for any transfer after the first arrives. The SDK allows one + // transfer at a time (a second start returns 0x8D03), so a single pair is + // sufficient. + std::atomic m_inFlightContentId{0}; + std::atomic m_inFlightFileId{0}; // Remote-transfer per-slot content lists (SDK-allocated; freed via // release_contents_info) and the download-in-progress flag the transfer From d7803626adfbc5e880d167a4cbe3981124373f4e Mon Sep 17 00:00:00 2001 From: jordlee Date: Thu, 13 Aug 2026 13:34:54 -0700 Subject: [PATCH 5/6] chore(mcp): regenerate schema.d.ts after the event-catalog correction api/openapi.yaml is the source of truth for mcp/src/api, so the transferProgress description change in the previous commit left schema.d.ts stale and would have failed the codegen-drift gate. Comment-only: one line, no type or tool changes. 71/71 MCP tests pass. Refs #47 Co-Authored-By: Claude Opus 5 --- mcp/src/api/schema.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcp/src/api/schema.d.ts b/mcp/src/api/schema.d.ts index 058deae..4bdcfce 100644 --- a/mcp/src/api/schema.d.ts +++ b/mcp/src/api/schema.d.ts @@ -911,7 +911,7 @@ export interface paths { * | `propertyChanged` | `OnPropertyChangedCodes` | `{cameraId, codes: [...]}` | One or more property values changed on the camera | * | `afStatus` | `OnWarning` (`CrWarningExt_AFStatus`) | `{state: "focused" \| "unlocked" \| "tracking", source?}` | Autofocus state changed | * | `downloadComplete` | `OnCompleteDownload` | `{cameraId, filename, savedPath}` | Auto-transferred image saved to host (remote mode + matching `still-image-store-destination`) | - * | `transferProgress` | `OnNotifyRemoteTransferResult` (or filesystem polling fallback) | `{cameraId, contentId, fileId, percent, savedPath?}` | Explicit file-pull progress (remote-transfer mode). On macOS V2.01 the SDK callback is unreliable; the server falls back to filesystem polling and emits synthetic events | + * | `transferProgress` | `OnNotifyRemoteTransferResult` (or filesystem polling fallback) | `{cameraId, contentId, fileId, percent, notify, savedPath?, filename?, synthetic?}` | Explicit file-pull progress (remote-transfer mode). `filename` duplicates `savedPath` and is retained for existing consumers — prefer `savedPath`. `synthetic: true` marks an event produced by the filesystem-polling fallback rather than the SDK callback; on SDK V2.02 (macOS/Linux) the real callback fires reliably and `synthetic` is absent | * | `contentsTransfer` | `OnContentsTransfer` | `{cameraId, files: [...]}` | Contents-transfer mode file list update (printable-ASCII filenames only) | * | `operationResult` | `OnCompleteOperation` | `{cameraId, operation, result}` | Generic SDK operation completed (e.g. control codes, non-shutter commands) | * | `error` | `OnError` | `{cameraId, code, message}` | SDK error occurred | From 18ab402b098938f1c1d5dfadf7a9de95ae138969 Mon Sep 17 00:00:00 2001 From: jordlee Date: Thu, 13 Aug 2026 13:37:19 -0700 Subject: [PATCH 6/6] docs: document the transferProgress payload, the 409 busy response, and the log query params Brings the docs in line with the behaviour changes in this PR (and satisfies the spec-docs sync gate, which api/openapi.yaml changes trip). - events: transferProgress now shows cameraId / contentId / fileId / savedPath, notes that filename is retained for existing consumers, and explains the synthetic flag rather than repeating the stale claim that the macOS callback is unreliable. - sd-card: document that the SDK permits one transfer at a time and that a concurrent request returns a retryable 409, distinct from the 0x8D02 400. - server: document the lines and level params properly, including clamping and fallback behaviour, and the new `returned` field. Refs #46, refs #47, refs #49 Co-Authored-By: Claude Opus 5 --- site/src/content/docs/web-api/events.mdx | 24 ++++++++++++++++++++--- site/src/content/docs/web-api/sd-card.mdx | 21 +++++++++++++++++++- site/src/content/docs/web-api/server.mdx | 12 ++++++++++-- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/site/src/content/docs/web-api/events.mdx b/site/src/content/docs/web-api/events.mdx index 0d411b8..b14c5bc 100644 --- a/site/src/content/docs/web-api/events.mdx +++ b/site/src/content/docs/web-api/events.mdx @@ -111,9 +111,27 @@ Only fires in `remote` connection mode with `still-image-store-destination` set ### transferProgress ```json -{"percent": 100, "notify": "0x20093", "filename": "/path/to/DSC09598.JPG"} +{ + "percent": 100, + "notify": "0x20093", + "cameraId": "CFCA6014E092", + "contentId": 131301, + "fileId": 1, + "savedPath": "/path/to/DSC09598.JPG", + "filename": "/path/to/DSC09598.JPG" +} ``` -Only fires in `remote-transfer` mode after explicit file pull. +Only fires in `remote-transfer` mode after an explicit file pull. + +`contentId` and `fileId` identify which pull completed — match them against the +values you passed to `POST .../files/{contentId}/{fileId}/download`. + +`filename` duplicates `savedPath` and is retained for existing consumers; prefer +`savedPath`. + +A `"synthetic": true` field marks an event produced by the server's +filesystem-polling fallback rather than the SDK callback. On SDK V2.02 the real +callback fires reliably and the field is absent. ### lutImportResult ```json @@ -149,7 +167,7 @@ data: {"state":"unlocked"} ``` event: transferProgress -data: {"percent":100,"notify":"0x20093","filename":"/path/to/DSC09598.JPG"} +data: {"percent":100,"notify":"0x20093","cameraId":"CFCA6014E092","contentId":131301,"fileId":1,"savedPath":"/path/to/DSC09598.JPG","filename":"/path/to/DSC09598.JPG"} ``` --- diff --git a/site/src/content/docs/web-api/sd-card.mdx b/site/src/content/docs/web-api/sd-card.mdx index 7d5ebd3..0bbc538 100644 --- a/site/src/content/docs/web-api/sd-card.mdx +++ b/site/src/content/docs/web-api/sd-card.mdx @@ -64,7 +64,26 @@ curl -X POST http://localhost:8080/api/cameras/D10F60149B0C/sd-card/slot/1/files -Listen for `transferProgress` SSE events to track download progress in `remote-transfer` mode. +Listen for `transferProgress` SSE events to track download progress in `remote-transfer` mode. Match the event's `contentId` / `fileId` against the ones you requested to identify which pull completed. + +### One transfer at a time + +The SDK permits a single RemoteTransfer operation at a time. A download, thumbnail +or screennail request issued while another is still in flight is rejected with: + +```json +HTTP 409 +{ + "success": false, + "retryable": true, + "message": "A transfer is already in progress on this camera, so the file download was not started. This is transient — retry once the current transfer completes (watch for a transferProgress event). SDK error 0x00008D03." +} +``` + +This is transient, not a failure of the requested file: wait for the in-flight +transfer's `transferProgress` event and retry. A `400` with `0x00008D02` is the +different case — the file identifiers are not valid for the current connection +mode. --- diff --git a/site/src/content/docs/web-api/server.mdx b/site/src/content/docs/web-api/server.mdx index f65fb00..863b960 100644 --- a/site/src/content/docs/web-api/server.mdx +++ b/site/src/content/docs/web-api/server.mdx @@ -90,8 +90,15 @@ curl http://localhost:8080/api/server/status ### Server logs -`GET /api/server/logs` returns buffered server log output. Query params: `lines` -(default 100), `level` (default `info`). +`GET /api/server/logs` returns buffered server log output. + +Query params: `lines` (default 100, clamped to the retained buffer) and `level` +(default `info`, one of `debug` / `info` / `warn` / `error` — entries at or above +the given level are returned). A non-numeric `lines` or an unrecognised `level` +falls back to the default rather than erroring. + +The response reports both `total` (everything retained) and `returned` (what +survived the filter), so a filtered response is not mistaken for data loss. @@ -113,6 +120,7 @@ curl http://localhost:8080/api/server/logs "message": "GET /api/server/status" } ], + "returned": 1, "total": 47 } ```