Drop the plain screens the chrome replaced, and test what ships - #16
Merged
Conversation
seedtool_render_screen3, seedtool_render_screen4 and seedtool_render_list are the text and list screens from before the nav chrome. Every screen that used them was migrated across #11, #12 and #14, and nothing in the firmware has called them since. Nothing said so. -Werror=unused-function only sees static functions, and these are public - declared in seedtool_render.h and seedtool_display.h, implemented in seedtool_render.c, seedtool_display.c and origo_sdl.c. To the compiler, any translation unit might still call them. This buys no space. The linker already dropped them: none of the three symbols appears in the built ELF, and the image is 303920 bytes before and after, to the byte. What it buys is 116 fewer lines for a reader who has to understand the whole tree to trust it, and the fix below. The self-test was the only caller left, and two of its four calls were the reason to do this rather than the obstacle. They drew the old list and said they were exercising "both arrows and the selection bar" - the arrows are that renderer's scroll indicator, and the shipping list uses a thumb instead. They were proving a widget the firmware had stopped sending, which is the same failure "Take the tick to the lists" already found once, in the opposite direction: a check that only knows how to see presence cannot see excess, and one that draws the wrong widget cannot see the right one. The other two calls are wire_order_is_big_endian, which pins the panel's byte order and is the one property the host can hold the hardware to. It paints a list to get the selection orange - the colour that actually came out blue on the device - so switching its renderer risked leaving the test green over an image without the colour it exists to check. Measured before switching: both renderers put 226 orange pixels on row 21, the row it inspects, and the test ends in `return orange_found`, so a wrong image fails it rather than passing hollowly. Seven comments named the removed functions to explain layout constants that are still in use. They are rewritten rather than deleted: the four rows still run 28 to 88 in steps of 20, and that still needs a reason, just not one that points at a function nobody can read any more. Verified with the host self-test, the 52 Python tests, the QR smoke test, the self-test under ASan and UBSan, and a firmware build the ELF audit passes at 303920 bytes. Flashed to a TTGO T-Display and the Origo menu confirmed still orange on the panel, which is the half of the byte-order question the host cannot answer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
seedtool_render_screen3,seedtool_render_screen4andseedtool_render_listare the text and list screens from before the nav chrome. Every screen that used them was migrated across #11, #12 and #14, and nothing in the firmware has called them since.Nothing said so.
-Werror=unused-functiononly seesstaticfunctions, and these are public — declared inseedtool_render.handseedtool_display.h, implemented inseedtool_render.c,seedtool_display.candorigo_sdl.c. To the compiler, any translation unit might still call them.This saves no space
The linker already dropped them. None of the three symbols appears in the built ELF, and the image is 303920 bytes before and after, to the byte.
What it buys is 116 fewer lines for a reader who has to understand the whole tree to trust it — and the test fix below, which is the real reason to do it.
The self-test was the only caller left
Two of its four calls were the point rather than the obstacle. They drew the old list and said they exercised "both arrows and the selection bar". Those arrows are that renderer's scroll indicator; the shipping list uses a thumb. They were proving a widget the firmware had stopped sending — the same failure "Take the tick to the lists and the paged screens too" already found once, from the other side: a check that only knows how to see presence cannot see excess, and one that draws the wrong widget cannot see the right one.
The one that needed care
The other two calls are
wire_order_is_big_endian, which pins the panel's byte order — the one property the host can hold the hardware to, and the reason the README says a colour right in the simulator can still be wrong on the board. It paints a list to obtain the selection orange, "the colour that actually came out blue".Switching its renderer risked leaving the test green over an image without the colour it exists to check. Measured before switching: both renderers put 226 orange pixels on row 21, the row it inspects, with black and white present in both. The test also ends in
return orange_found, so a wrong image fails it rather than passing hollowly.Comments
Seven comments named the removed functions to explain layout constants that are still in use. They are rewritten, not deleted: the four rows still run 28 to 88 in steps of 20, and that still needs a reason — just not one pointing at a function nobody can read any more.
Verification
Host self-test, 52 Python tests, QR smoke test, the self-test under ASan and UBSan, and a firmware build the ELF audit passes at 303920 bytes.
Flashed to a TTGO T-Display, and the Origo menu confirmed still orange on the panel. That is the half of the byte-order question the host cannot answer, and the menu is the screen whose renderer changed.