[Refactor] Human-friendly splash definition

[kdmukai]

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.

Changes made to:

• Code

Did you build the code and tested on device?

• Yes, build and tested on

What is the purpose of this pull request?

• Other: light refactor

[odudex]

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

[tadeubas]

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)

[odudex]

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

[kdmukai]

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

[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]

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.

[odudex]

Almost there. It still has a centralizarion issue.

[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

[tadeubas]

@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 ☝️ 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]

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]

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 🤣

[kdmukai]

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!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.37%. Comparing base (6421209) to head (3b385bd).
⚠️ 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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[qlrd]

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

[qlrd]

Some nits. Also, THIN_SPACE isn't used anymore, and this will raise a warning by pylint, so it need to be removed from src/krux/display:

src/krux/display.py:27:0: W0611: Unused THIN_SPACE imported from settings (unused-import)

[qlrd]

This will throw an warning on pylint:

src/krux/display.py:55:0: C0103: Constant name "human_friendly_splash" doesn't conform to UPPER_CASE naming style (invalid-name)

Suggested change

	human_friendly_splash = """
	SPLASH = """

[qlrd]

Suggested change

	SPLASH = ["{:<9}".format(row) for row in human_friendly_splash.split("\n") if row != '']
	SPLASH = ["{:<9}".format(row) for row in SPLASH.split("\n") if row != '']