feat: add Stackbit 1248 vertical layout for backup display#847
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #847 +/- ##
===========================================
+ Coverage 97.31% 97.32% +0.01%
===========================================
Files 83 83
Lines 10632 10798 +166
===========================================
+ Hits 10346 10509 +163
- Misses 286 289 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
IMO this is ready to review |
There was a problem hiding this comment.
cACK
IMO you can make 1 square per page. While in Amigo you have a big screen, users will already flip the metal backup. It's good for user review to confirm the dots. Also maybe its too smal for m5 to use more than 1 per page (in this geometry).
I think will be good refactor some lines so we can reuse them in both this and #839
| n_word_cols = 2 | ||
| n_cols_per_word = 4 | ||
| word_col_gap = 4 | ||
| row_gap = 3 | ||
| header_h = FONT_HEIGHT | ||
|
|
||
| label_w = FONT_WIDTH | ||
| x_start = MINIMAL_PADDING | ||
| right_pad = MINIMAL_PADDING | ||
|
|
||
| available_w = ( | ||
| self.ctx.display.width() - x_start - label_w - word_col_gap - right_pad | ||
| ) | ||
| cell_w = available_w // (n_word_cols * n_cols_per_word) | ||
|
|
||
| n_rows = (len(words_list) + n_word_cols - 1) // n_word_cols | ||
| available_h = self.ctx.display.height() - y_start | ||
| cell_h = (available_h - n_rows * header_h - (n_rows - 1) * row_gap) // ( | ||
| n_rows * 4 | ||
| ) | ||
| cell_h = min(cell_h, FONT_HEIGHT) | ||
|
|
||
| grid_w = n_cols_per_word * cell_w | ||
| row_total_h = header_h + 4 * cell_h | ||
|
|
||
| dot_size = max(min(cell_w, cell_h) - 6, 1) | ||
| radius = dot_size // 2 | ||
|
|
||
| x_label = x_start | ||
| x_grid_left = x_label + label_w | ||
| x_grid_right = x_grid_left + grid_w + word_col_gap |
There was a problem hiding this comment.
All those variables are necessary? IMO this could be simplified with constants.
There was a problem hiding this comment.
These variables are used for layout calculation on different screen sizes! Happy to simplify if you have specific suggestions :)
There was a problem hiding this comment.
My suggestion requires adding some constants at the beginning (untested):
N_WORD_COLS = 2
WORD_COL_GAP = 4 # also matches `n_cols_per_word` in your commit
ROW_GAP = 3| n_word_cols = 2 | |
| n_cols_per_word = 4 | |
| word_col_gap = 4 | |
| row_gap = 3 | |
| header_h = FONT_HEIGHT | |
| label_w = FONT_WIDTH | |
| x_start = MINIMAL_PADDING | |
| right_pad = MINIMAL_PADDING | |
| available_w = ( | |
| self.ctx.display.width() - x_start - label_w - word_col_gap - right_pad | |
| ) | |
| cell_w = available_w // (n_word_cols * n_cols_per_word) | |
| n_rows = (len(words_list) + n_word_cols - 1) // n_word_cols | |
| available_h = self.ctx.display.height() - y_start | |
| cell_h = (available_h - n_rows * header_h - (n_rows - 1) * row_gap) // ( | |
| n_rows * 4 | |
| ) | |
| cell_h = min(cell_h, FONT_HEIGHT) | |
| grid_w = n_cols_per_word * cell_w | |
| row_total_h = header_h + 4 * cell_h | |
| dot_size = max(min(cell_w, cell_h) - 6, 1) | |
| radius = dot_size // 2 | |
| x_label = x_start | |
| x_grid_left = x_label + label_w | |
| x_grid_right = x_grid_left + grid_w + word_col_gap | |
| available_w = ( | |
| self.ctx.display.width() - (MINIMAL_PADDING * 2) - FONT_WIDTH - WORD_COL_GAP | |
| ) | |
| cell_w = available_w // (N_WORD_COLS * WORD_COL_GAP) | |
| n_rows = (len(words_list) + N_WORD_COLS - 1) // N_WORD_COLS | |
| available_h = self.ctx.display.height() - y_start | |
| cell_h = (available_h - n_rows * FONT_HEIGHT - (n_rows - 1) * ROW_GAP) // ( | |
| n_rows * 4 | |
| ) | |
| cell_h = min(cell_h, FONT_HEIGHT) | |
| grid_w = WORD_COL_GAP * cell_w | |
| row_total_h = FONT_HEIGHT + 4 * cell_h | |
| dot_size = max(min(cell_w, cell_h) - 6, 1) | |
| radius = dot_size // 2 | |
| x_label = MINIMAL_PADDING | |
| x_grid_left = x_label + FONT_WIDTH | |
| x_grid_right = x_grid_left + grid_w + WORD_COL_GAP |
Or something like that, is a suggestion, try yourself :)
|
Also, you could do a more cleaner history: pick 2b7e8df9
fixup e521f8d3 # or maybe squash, if you wanna |
This commit add a feature commented by @jaonoctus on a [\selfcustody#847 comment](https://github.com/selfcustody/krux/pull/847/changes#r3029819808), where an unecessary review about multiple lines could be avoided automatically if we apply the `mccabe` plugin to `pylint`. Tested with some weird complex code and catch nice.
This commit add a feature commented by @jaonoctus on a [\selfcustody#847 comment](https://github.com/selfcustody/krux/pull/847/changes#r3029819808), where an review about multiple lines review could be avoided automatically if we apply the `mccabe` plugin to `pylint`. E.g, visually complexity could lead humans to interpret some lines as complex given a experience of how code should be or not to be; some standars could say that 25 lines could be the limit of a readability. But a cyclomatic complexity on a function could be used as a "how real complex is the function" and "it should be really refactored?". Tested with `max-complexity=13` and `max-complexity=20` in tags `v26.03.0`, branches `main`, `develop` and `feat/stackbit-1248-vertical`. For more details about values a wikipedia short ref: * 1–10: Simple procedure: `little risk`; * 11–20: More complex: `moderate risk`; * 21–50: Complex: `high risk`. * > 50: Untestable code: `very high risk`;
This commit add a feature commented by @jaonoctus on a [\selfcustody#847 comment](https://github.com/selfcustody/krux/pull/847/changes#r3029819808), where an review about multiple lines review could be avoided automatically if we apply the `mccabe` plugin to `pylint`. E.g, visually complexity could lead humans to interpret some lines as complex given a experience of how code should be or not to be; some standars could say that 25 lines could be the limit of a readability. But a cyclomatic complexity on a function could be used as a "how real complex is the function" and "it should be really refactored?". Tested with `max-complexity=13` and `max-complexity=20` in tags `v26.03.0`, branches `main`, `develop` and `feat/stackbit-1248-vertical`. For more details about values a wikipedia short ref: * 1–10: Simple procedure: `little risk`; * 11–20: More complex: `moderate risk`; * 21–50: Complex: `high risk`. * > 50: Untestable code: `very high risk`;
This commit add a feature commented by @jaonoctus on a [\selfcustody#847 comment](https://github.com/selfcustody/krux/pull/847/changes#r3029819808), where an review about multiple lines review could be avoided automatically if we apply the `mccabe` plugin to `pylint`. E.g, visually complexity could lead humans to interpret some lines as complex given a experience of how code should be or not to be; some standars could say that 25 lines could be the limit of a readability. But a cyclomatic complexity on a function could be used as a "how real complex is the function" and "it should be really refactored?". Tested with `max-complexity=13` and `max-complexity=20` in tags `v26.03.0`, branches `main`, `develop` and `feat/stackbit-1248-vertical`. For more details about values a wikipedia short ref: * 1–10: Simple procedure: `little risk`; * 11–20: More complex: `moderate risk`; * 21–50: Complex: `high risk`. * > 50: Untestable code: `very high risk`;
0d0e0ee to
bd31de8
Compare
This commit add a feature commented by @jaonoctus on a [\selfcustody#847 comment](https://github.com/selfcustody/krux/pull/847/changes#r3029819808), where an review about multiple lines review could be avoided automatically if we apply the `mccabe` plugin to `pylint`. E.g, visually complexity could lead humans to interpret some lines as complex given a experience of how code should be or not to be; some standars could say that 25 lines could be the limit of a readability. But a cyclomatic complexity on a function could be used as a "how real complex is the function" and "it should be really refactored?". Tested with `max-complexity=13` and `max-complexity=20` in tags `v26.03.0`, branches `main`, `develop` and `feat/stackbit-1248-vertical`. For more details about values a wikipedia short ref: * 1–10: Simple procedure: `little risk`; * 11–20: More complex: `moderate risk`; * 21–50: Complex: `high risk`. * > 50: Untestable code: `very high risk`; For now, this commit found 7 `too-complex` functions on source code base.
This commit add a feature commented by @jaonoctus on a [\selfcustody#847 comment](https://github.com/selfcustody/krux/pull/847/changes#r3029819808), where an review about multiple lines review could be avoided automatically if we apply the `mccabe` plugin to `pylint`. E.g, visually complexity could lead humans to interpret some lines as complex given a experience of how code should be or not to be; some standars could say that 25 lines could be the limit of a readability. But a cyclomatic complexity on a function could be used as a "how real complex is the function" and "it should be really refactored?". Tested with `max-complexity=13` and `max-complexity=20` in tags `v26.03.0`, branches `main`, `develop` and `feat/stackbit-1248-vertical`. For more details about values a wikipedia short ref: * 1–10: Simple procedure: `little risk`; * 11–20: More complex: `moderate risk`; * 21–50: Complex: `high risk`. * > 50: Untestable code: `very high risk`; For now, this commit found 7 `too-complex` functions on source code base.
dd5174d to
c09ce04
Compare
Did the rebase, but idk why the ci is blocking my commit name on "i18n"... Tried to tie everything here |
you found a bug on While, you can |
c09ce04 to
8a3cc69
Compare
We did it!!! Thanks! Everything check passed! |
3d3a27e to
8a3cc69
Compare
|
@odudex happy to share the video of this PR running on my yahboom :) thx @joaozinhom for your help! WhatsApp.Video.2026-05-05.at.16.42.38.mp4 |
|
Ok, before merging, can you add a CHANGELOG entry and documentation with a screenshot of the vertical option (generated through simulator/sequences script update)? |
|
@odudex Done! Added CHANGELOG entry, documentation with screenshots of Vertical layouts, also a tiny description, and updated the simulator sequences script to generate the new screenshots. |
Add grouped and compact export methods.
Test grouped/compact layouts + 24-word pagination.
bccdab0 to
8b2881d
Compare
8b2881d to
22b4cb3
Compare
a month late, I like this feature (while I don't know where to buy these plates). Currently looking into the follow-up pr 862. |
What is this PR for?
This PR adds vertical layout support for Stackbit 1248 backup display.
Currently, the Stackbit 1248 grid only supports the horizontal layout (rows = digits, columns = weights 1/2/4/8). This adds a menu to choose between "Standard" (horizontal) and "Vertical" (rows = weights, columns = digits), which is essentially the transposition of the grid.
This PR covers the backup/display side only. The input/loading side will be addressed in a separate PR as discussed with maintainers.
Close backup part of #834
Changes made to:
Did you build the code and tested on device?
What is the purpose of this pull request?
Screenshots (not the same words on the devices):
After selecting Stackbit 1248 on menu:
Choosing Standard:
Choosing Vertical: