krux icon indicating copy to clipboard operation
krux copied to clipboard

[Refactor] Human-friendly splash definition

Open kdmukai opened this issue 1 month ago • 13 comments

What is this PR for?

Very minor refactor. Have a few changes coming but trying to keep each one as simple and as isolated as possible.

While working on some screensaver experiments, it seemed like the SPLASH definition was unnecessarily complex.

  • Everything is rendered using fixed-width chars anyway so a THIN_SPACE shouldn't be any different than a normal space char, right?
  • Makes the code look cooler. 😂
  • List comprehension takes care of padding / formatting.

Question: Is the currently used version of MicroPython okay with the f"{row:9}" f-string formatting?

This change is running fine in the emulator for maixpy_amigo but I haven't tried it on an actual device yet. I can't discern any visual difference vs the original definition.

Screenshot 2025-11-11 at 10 26 47 AM

Changes made to:

  • [x] Code

Did you build the code and tested on device?

  • [ ] Yes, build and tested on

What is the purpose of this pull request?

  • [x] Other: light refactor

kdmukai avatar Nov 11 '25 16:11 kdmukai

Nice! I think it can work, as long you replace the fstrings with a "{}".format ... Unfortunately our micropython does not support ftrings.

odudex avatar Nov 11 '25 17:11 odudex

Hey @kdmukai! 👋 Great to see your contribution! Just a quick note - all PRs should target the develop branch. Also, make sure to build and test it on the device - you’ll notice it currently breaks on MicroPython 😅 but it should be an easy fix!

If you are using linux it is as simple as running ./krux build maixpy_tzt for the build process and then ./krux flash maixpy_tzt to put the firmware onto the device and see results (just change the name of the device you have). To connect to the terminal of the device use: poetry run poe terminal and you should see the errors (most of the time we catch errors with try: but the majority will fall here so just comment the try: and catch and you should see in terminal the exact line where error ocurred)

tadeubas avatar Nov 11 '25 17:11 tadeubas

@tadeubas , do you remember what's the reason we switched to THIN_SPACE?

odudex avatar Nov 11 '25 17:11 odudex

I'll flip this to DRAFT until those TODO items are resolved.

kdmukai avatar Nov 11 '25 17:11 kdmukai

I never know python syntax nuance.

This works as expected: "{:9}".format(row)

But seems like the "proper" way to format it is: "{:<9}".format(row)

🤷‍♂️

kdmukai avatar Nov 11 '25 17:11 kdmukai

Compilation against 3b385bd for the maixpy_wonder_k worked (via my remote machine at home).

19e0785cc8a90fc3c8feae7a346f66e63ea3231b6e6c03f1813d66b079939a36
Successfully copied 1.69MB to /home/kdmukai/krux/build/firmware.bin
Successfully copied 863kB to /home/kdmukai/krux/build/kboot.kfpkg
extract-firmware

Flashing to my device will be a challenge since Docker is trashed on my dev laptop.

kdmukai avatar Nov 11 '25 17:11 kdmukai

Almost there. It still has a centralizarion issue.

20251111_144627.jpg

odudex avatar Nov 11 '25 17:11 odudex

Flashing to my device will be a challenge since Docker is trashed on my dev laptop.

If you manage to have access to ktool-mac and kboot.kfpkg inside build folder. You can copy them to local machine and flash the device. Put them in the same folder and run: ktool-mac -B dan -b 1500000 kboot.kfpkg

odudex avatar Nov 11 '25 17:11 odudex

@tadeubas , do you remember what's the reason we switched to THIN_SPACE?

No I don't, just found this was the old one but used to occupy more ram: https://github.com/selfcustody/krux/blame/a99cf184dc587de131f2c28f798548f3debd1d3a/src/boot.py#L44

Than changed to this compressed one: https://github.com/selfcustody/krux/blame/cdfeec1d079ccbf9badac9783f3b01eac46aaebd/src/boot.py#L34

Than removed from boot.py and added to display.py for reuse with screensaver: https://github.com/selfcustody/krux/blame/268cc733aba81855983ecbca6a66a90cac21930f/src/krux/display.py#L55

The above :point_up: on the line before my comment says "# Splash will use horizontally-centered text plots. Uses Thin spaces to help with alignment", so maybe it is necessary or was necessary for alignment

tadeubas avatar Nov 11 '25 18:11 tadeubas

Flashing to my device will be a challenge since Docker is trashed on my dev laptop.

The end of the script copy those files generated inside the docker machine to the folder build/ where you have the krux script

tadeubas avatar Nov 11 '25 18:11 tadeubas

Almost there. It still has a centralizarion issue.

Maybe this is the reason I used thin-spaces? to store less chars as possible in ram and to have it beautiful drawn at the same time?

Not sure if @kdmukai has run into this before, but we usually optimize things a lot since we’re working with pretty limited resources. Maybe it is not worthy to change this code to be more human readable :rofl:

tadeubas avatar Nov 11 '25 18:11 tadeubas

but we usually optimize things a lot since we’re working with pretty limited resources.

I've definitely been spoiled by the Pi Zero's 512MB of RAM!

kdmukai avatar Nov 11 '25 18:11 kdmukai

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 97.37%. Comparing base (6421209) to head (3b385bd). :warning: Report is 13 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #765      +/-   ##
===========================================
+ Coverage    97.10%   97.37%   +0.27%     
===========================================
  Files           82       83       +1     
  Lines        10422    10526     +104     
===========================================
+ Hits         10120    10250     +130     
+ Misses         302      276      -26     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 13 '25 11:11 codecov[bot]

Needs rebase. The last commit on your branch before your commits is 64212093. The correct should be (at the moment) 573de4336e9d0e517a46d469d1114f917cf35280.

qlrd avatar Dec 05 '25 17:12 qlrd