fix(screenshot): drop C-style cast UB, fix ERR check order

handleScreenshotRequest() reinterpret-cast every DisplayDriver* first to
SH1106Display* then to SSD1306Display*; both casts always produce a
non-null pointer, so the SSD1306 fallback was unreachable and on any
SSD1306 build we called SH1106::getBuffer() on a SSD1306 object — UB
that only happened to work because both backing classes share an
Adafruit_GFX layout at offset zero.

Replace the cast hack with virtual getBuffer()/getBufferSize() on
DisplayDriver, overridden in SH1106Display and SSD1306Display. MyMesh
no longer needs to know about either concrete type. const-correct the
buffer pointer and widen bufferSize to uint16_t while passing through.

Also fix the matching Python decoder: it sanity-checked length before
dispatching on resp_code, so a 2-byte RESP_CODE_ERR frame was reported
as "Frame too short" instead of the actual device error. Move the
length check after the resp_code dispatch and log the error code byte.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Jakub
2026-05-28 22:58:26 +02:00
parent 6687d2b0e6
commit 5dd035d309
6 changed files with 76 additions and 83 deletions

View File

@@ -7,8 +7,6 @@
#ifdef DISPLAY_CLASS
#include "helpers/ui/DisplayDriver.h"
#include "helpers/ui/SH1106Display.h"
#include "helpers/ui/SSD1306Display.h"
#include "UITask.h"
#endif
@@ -2083,29 +2081,13 @@ void MyMesh::handleScreenshotRequest() {
writeErrFrame(ERR_CODE_UNSUPPORTED_CMD);
return;
}
uint8_t* buffer = nullptr;
int bufferSize = 0;
// Try SH1106Display first (used by Wio L1)
// Use C-style cast since RTTI is disabled (-fno-rtti)
SH1106Display* sh1106 = (SH1106Display*)display;
if (sh1106) {
buffer = sh1106->getBuffer();
bufferSize = (display->width() * display->height()) / 8;
} else {
// Fallback: try SSD1306Display
SSD1306Display* ssd1306 = (SSD1306Display*)display;
if (ssd1306) {
buffer = ssd1306->getBuffer();
bufferSize = (display->width() * display->height()) / 8;
}
}
const uint8_t* buffer = display->getBuffer();
uint16_t bufferSize = display->getBufferSize();
if (buffer && bufferSize > 0) {
sendScreenshotResponse(display, buffer, bufferSize);
} else {
// No supported display type
writeErrFrame(ERR_CODE_UNSUPPORTED_CMD);
}
#else
@@ -2113,26 +2095,28 @@ void MyMesh::handleScreenshotRequest() {
#endif
}
void MyMesh::sendScreenshotResponse(DisplayDriver* display, uint8_t* buffer, int bufferSize) {
// MAX_FRAME_SIZE is 172 bytes, but we need to send 1026 bytes (3 header + 1024 buffer)
// We need to split into multiple frames
// Frame format: RESP_CODE_SCREENSHOT, width, height, chunk_index, total_chunks, [chunk_data...]
const int MAX_DATA_PER_FRAME = MAX_FRAME_SIZE - 5; // 5 bytes header
void MyMesh::sendScreenshotResponse(DisplayDriver* display, const uint8_t* buffer, uint16_t bufferSize) {
// Frame format (5-byte header): RESP_CODE_SCREENSHOT, width, height,
// chunk_idx, total_chunks, [chunk_data...]
// width/height fit in uint8_t for all displays this firmware targets
// (SH1106 128×64, SSD1306 128×64); the framebuffer is page-based 1bpp
// column-major, byte_idx = page*width + col.
const int HEADER_SIZE = 5;
const int MAX_DATA_PER_FRAME = MAX_FRAME_SIZE - HEADER_SIZE;
int totalChunks = (bufferSize + MAX_DATA_PER_FRAME - 1) / MAX_DATA_PER_FRAME;
for (int chunkIdx = 0; chunkIdx < totalChunks; chunkIdx++) {
int i = 0;
out_frame[i++] = RESP_CODE_SCREENSHOT;
out_frame[i++] = display->width();
out_frame[i++] = display->height();
out_frame[i++] = chunkIdx;
out_frame[i++] = totalChunks;
int chunkSize = min(MAX_DATA_PER_FRAME, bufferSize - chunkIdx * MAX_DATA_PER_FRAME);
out_frame[i++] = (uint8_t)display->width();
out_frame[i++] = (uint8_t)display->height();
out_frame[i++] = (uint8_t)chunkIdx;
out_frame[i++] = (uint8_t)totalChunks;
int chunkSize = min(MAX_DATA_PER_FRAME, (int)bufferSize - chunkIdx * MAX_DATA_PER_FRAME);
memcpy(&out_frame[i], buffer + chunkIdx * MAX_DATA_PER_FRAME, chunkSize);
i += chunkSize;
_serial->writeFrame(out_frame, i);
}
}

View File

@@ -229,7 +229,7 @@ private:
bool isValidClientRepeatFreq(uint32_t f) const;
#ifdef ENABLE_SCREENSHOT
void handleScreenshotRequest();
void sendScreenshotResponse(DisplayDriver* display, uint8_t* buffer, int bufferSize);
void sendScreenshotResponse(DisplayDriver* display, const uint8_t* buffer, uint16_t bufferSize);
#endif
UITask* getUITask() { return (UITask*)_ui; }

View File

@@ -247,4 +247,12 @@ public:
virtual void setDisplayRotation(uint8_t rot) { } // 0-3, no-op for fixed-orientation displays
virtual void setFullRefreshInterval(uint8_t n) { } // e-ink: do full refresh every n partial refreshes (0=never)
virtual void endFrame() = 0;
#ifdef ENABLE_SCREENSHOT
// Screenshot support — virtual so we never reinterpret_cast the concrete
// display type (RTTI is disabled). Drivers that can dump their framebuffer
// override these; default returns nullptr/0 so the caller fails cleanly.
virtual const uint8_t* getBuffer() { return nullptr; }
virtual uint16_t getBufferSize() { return 0; }
#endif
};

View File

@@ -60,7 +60,7 @@ public:
void endFrame() override;
#ifdef ENABLE_SCREENSHOT
// Screenshot support
uint8_t* getBuffer() { return display.getBuffer(); }
const uint8_t* getBuffer() override { return display.getBuffer(); }
uint16_t getBufferSize() override { return (uint16_t)((width() * height()) / 8); }
#endif
};

View File

@@ -47,7 +47,7 @@ public:
void endFrame() override;
#ifdef ENABLE_SCREENSHOT
// Screenshot support
uint8_t* getBuffer() { return display.getBuffer(); }
const uint8_t* getBuffer() override { return display.getBuffer(); }
uint16_t getBufferSize() override { return (uint16_t)((width() * height()) / 8); }
#endif
};

View File

@@ -113,55 +113,56 @@ def receive_screenshot(ser, timeout=5):
frame = read_frame(ser)
if frame is None:
continue
if len(frame) < 5:
print(f"Error: Frame too short: {len(frame)} bytes")
return None
if len(frame) < 1:
continue
resp_code = frame[0]
if resp_code == RESP_CODE_SCREENSHOT:
w = frame[1]
h = frame[2]
chunk_idx = frame[3]
num_chunks = frame[4]
chunk_data = frame[5:]
if chunk_idx == 0:
width = w
height = h
total_chunks = num_chunks
buffer_data = bytearray()
received_chunks = 0
if width is None or width != w or height != h:
print(f"Error: Inconsistent dimensions in chunk {chunk_idx}")
return None
if total_chunks is None or total_chunks != num_chunks:
print(f"Error: Inconsistent chunk count in chunk {chunk_idx}")
return None
buffer_data.extend(chunk_data)
received_chunks += 1
if received_chunks >= total_chunks:
expected_size = (width * height) // 8
if len(buffer_data) == expected_size:
return bytes(buffer_data), width, height
else:
print(
f"Error: Expected {expected_size} bytes, got {len(buffer_data)}"
)
return None
elif resp_code == RESP_CODE_ERR:
print("Error: Device returned error")
if resp_code == RESP_CODE_ERR:
err = frame[1] if len(frame) > 1 else 0xFF
print(f"Error: Device returned error code 0x{err:02x}")
return None
else:
if resp_code != RESP_CODE_SCREENSHOT:
continue
if len(frame) < 5:
print(f"Error: Screenshot frame too short: {len(frame)} bytes (need >= 5)")
return None
w = frame[1]
h = frame[2]
chunk_idx = frame[3]
num_chunks = frame[4]
chunk_data = frame[5:]
if chunk_idx == 0:
width = w
height = h
total_chunks = num_chunks
buffer_data = bytearray()
received_chunks = 0
if width is None:
print(f"Error: Missed chunk 0 (first chunk seen: {chunk_idx})")
return None
if width != w or height != h:
print(f"Error: Inconsistent dimensions in chunk {chunk_idx}")
return None
if total_chunks != num_chunks:
print(f"Error: Inconsistent chunk count in chunk {chunk_idx}")
return None
buffer_data.extend(chunk_data)
received_chunks += 1
if received_chunks >= total_chunks:
expected_size = (width * height) // 8
if len(buffer_data) != expected_size:
print(f"Error: Expected {expected_size} bytes, got {len(buffer_data)}")
return None
return bytes(buffer_data), width, height
print(
f"Error: Timeout waiting for screenshot (received {received_chunks}/{total_chunks or '?'} chunks)"
)