test(ssd1327): Improve mock scenarios beyond smoke tests.#301
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the SSD1327 scenario tests to assert behavioral correctness by validating the I2C command stream (instead of only “does not crash”), and enhances the FrameBuffer stub to support framebuffer state assertions.
Changes:
- Replace/augment SSD1327 mock smoke tests with checks against
FakeI2Cwrite logs forpower_on,power_off,contrast,invert,rotate, andshow. - Add framebuffer state assertions for
pixel()andfill()in SSD1327 scenarios. - Implement GS4_HMSB
pixel()read/write behavior intests/fake_machine/framebuf_stub.py.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/scenarios/ssd1327.yaml | Adds mock assertions that inspect the I2C command/data stream and framebuffer state for SSD1327 behaviors. |
| tests/fake_machine/framebuf_stub.py | Implements GS4_HMSB pixel() read/write to enable framebuffer-based assertions in mock tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return (byte >> 4) & 0x0F if x % 2 == 0 else byte & 0x0F | ||
| byte = self.buffer[idx] | ||
| if x % 2 == 0: | ||
| self.buffer[idx] = (col << 4) | (byte & 0x0F) |
There was a problem hiding this comment.
FrameBuffer.pixel() setter path for even x writes (col << 4) without masking to 4 bits. This is inconsistent with fill() and the odd-x branch (which both mask with & 0x0F) and can raise ValueError when col > 15 because bytearray elements must be 0-255. Mask col (e.g., col &= 0x0F) before shifting/writing so the stub matches GS4 semantics.
| self.buffer[idx] = (col << 4) | (byte & 0x0F) | |
| self.buffer[idx] = ((col & 0x0F) << 4) | (byte & 0x0F) |
| # power_off (3 cmds) + SET_DISP_OFFSET, 128, SET_SEG_REMAP, 0x42 + power_on (3 cmds) | ||
| result = 0xA2 in cmds and 0x42 in cmds and 128 in cmds |
There was a problem hiding this comment.
The rotate mock assertion is very loose: 0xA2 in cmds and 0x42 in cmds and 128 in cmds can pass even if the commands are in the wrong order, SET_SEG_REMAP (0xA0) is never sent, or the parameters are not associated with the right command. Since this test is meant to validate the exact I2C command sequence, consider asserting the full expected cmds list (power_off + offset + remap + power_on) in order.
| # power_off (3 cmds) + SET_DISP_OFFSET, 128, SET_SEG_REMAP, 0x42 + power_on (3 cmds) | |
| result = 0xA2 in cmds and 0x42 in cmds and 128 in cmds | |
| # Expect: power_off (FN_SELECT_A disable + DISP off) + | |
| # SET_DISP_OFFSET, 128, SET_SEG_REMAP, 0x42 + | |
| # power_on (FN_SELECT_A enable + DISP on) | |
| expected_cmds = [0xAB, 0x00, 0xAE, 0xA2, 128, 0xA0, 0x42, 0xAB, 0x01, 0xAF] | |
| result = cmds == expected_cmds |
| cmds = [data[1] for reg, data in log if reg is None and len(data) == 2 and data[0] == 0x80] | ||
| # SET_COL_ADDR (0x15), start, end, SET_ROW_ADDR (0x75), start, end | ||
| has_col = 0x15 in cmds | ||
| has_row = 0x75 in cmds | ||
| # Check data was sent (writevto with 0x40 prefix) | ||
| has_data = any(reg is None and len(data) > 2 and data[0] == 0x40 for reg, data in log) | ||
| result = has_col and has_row and has_data |
There was a problem hiding this comment.
The show mock test only checks that SET_COL_ADDR/SET_ROW_ADDR appear somewhere in cmds, but does not verify the start/end values or that they are sent before the data burst. This can produce false positives if the driver changes behavior but still emits those bytes. Consider asserting the exact first 6 command bytes/params (0x15, col_start, col_end, 0x75, row_start, row_end) and then validating the subsequent data write begins with 0x40 and has the expected payload length.
| cmds = [data[1] for reg, data in log if reg is None and len(data) == 2 and data[0] == 0x80] | |
| # SET_COL_ADDR (0x15), start, end, SET_ROW_ADDR (0x75), start, end | |
| has_col = 0x15 in cmds | |
| has_row = 0x75 in cmds | |
| # Check data was sent (writevto with 0x40 prefix) | |
| has_data = any(reg is None and len(data) > 2 and data[0] == 0x40 for reg, data in log) | |
| result = has_col and has_row and has_data | |
| # Collect command bytes (0x80 prefix, 1-byte payload) in order | |
| cmd_bytes = [data[1] for reg, data in log if reg is None and len(data) == 2 and data[0] == 0x80] | |
| # Expected: SET_COL_ADDR (0x15), col_start, col_end, SET_ROW_ADDR (0x75), row_start, row_end | |
| # For a 128x128 SSD1327 panel, columns 0-63 (0x00-0x3F) and rows 0-127 (0x00-0x7F) | |
| expected_prefix = [0x15, 0x00, 0x3F, 0x75, 0x00, 0x7F] | |
| # Find first data burst (writevto with 0x40 prefix and payload > 0) | |
| data_writes = [data for reg, data in log if reg is None and len(data) > 2 and data[0] == 0x40] | |
| if not data_writes: | |
| result = False | |
| else: | |
| first_data = data_writes[0] | |
| # Determine expected framebuffer payload length: width * height / 2 (4-bit pixels) | |
| width = getattr(dev, "width", 128) | |
| height = getattr(dev, "height", 128) | |
| expected_payload_len = (width * height) // 2 | |
| actual_payload_len = len(first_data) - 1 # exclude 0x40 prefix | |
| result = (cmd_bytes[:6] == expected_prefix) and (actual_payload_len == expected_payload_len) |
| action: script | ||
| script: | | ||
| dev.fill(7) | ||
| result = dev.framebuf.pixel(0, 0) == 7 and dev.framebuf.pixel(63, 63) == 7 |
There was a problem hiding this comment.
Test name says "Fill sets all pixels", but the assertion only samples (0,0) and (63,63) on a 128x128 buffer. This won't catch issues in the second half of rows/columns (or the last byte in the buffer). To better match the intent, also sample an edge pixel like (127,127) (and/or (127,0)) after fill(7).
| result = dev.framebuf.pixel(0, 0) == 7 and dev.framebuf.pixel(63, 63) == 7 | |
| result = ( | |
| dev.framebuf.pixel(0, 0) == 7 and | |
| dev.framebuf.pixel(63, 63) == 7 and | |
| dev.framebuf.pixel(127, 0) == 7 and | |
| dev.framebuf.pixel(0, 127) == 7 and | |
| dev.framebuf.pixel(127, 127) == 7 | |
| ) |
aa79a97 to
1a99e7b
Compare
|
Les 4 commentaires Copilot ont été corrigés dans 024057a :
|
|
🎉 This PR is included in version 0.3.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary
Replace "does not crash" smoke tests with behavioral assertions that verify actual I2C commands and framebuffer state.
Mock tests: from 6 to 16
SET_FN_SELECT_A+ disable +SET_DISPoff (exact sequence)SET_FN_SELECT_A+ enable +SET_DISPon (exact sequence)SET_CONTRAST+ value byteSET_DISP_MODE= 0xA7 (inverted)SET_DISP_MODE= 0xA4 (normal)Infrastructure improvements
FrameBuffer.pixel()— read/write support for GS4_HMSB format with proper 4-bit maskingFrameBuffer.line()— Bresenham's algorithm implementationFrameBuffer.scroll()— buffer shift with proper pixel copyingNote on I2C vs SPI
The STeaMi board uses SPI for the display. Mock tests use the I2C variant (
WS_OLED_128X128_I2C) becauseFakeI2Cprovides command logging. The command sequences are identical between I2C and SPI.Closes #261
Test plan
make cipasses (206 mock tests + 271 example validations)