Draft
Conversation
This was referenced Oct 24, 2025
Contributor
|
Nice showcase. The right one certainly does look worse. |
Contributor
Author
|
@macpijan and is much, much slower. Another video (kinda hard to see what I'm doing, but I'm testing footer options): Screencast.From.2025-10-24.15-20-20.mp4 |
Contributor
|
On which HW this is tested? Can it be reproduced locally on the host machine, something like examples/demo.sh in the tui lib? It was drawing super fast there IIRC. |
Contributor
Author
That was real DTS running on QEMU, but it should work anywhere you can boot DTS. |
Contributor
|
I take it as a |
Contributor
Author
No, you can't (easily) run it on host machine without building (you need to build one with yq support) and running DTS in QEMU |
|
The menu drawing slowdown is due to tui_register_refresh_callback stop_trace_logging
tui_register_post_refresh_callback start_trace_loggingPost refresh callbacks would be called after |
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
Contributor
Author
|
The only tests that failed were ones that verify profiles, due to --- /tmp/robotframework-dts-profile\t2025-10-27 12:14:48.418321347 +0100
+++ "/home/miwanicki/projects/open-source-firmware-validation/dts/profiles/novacustom-v560tnd UEFI Update - DCR.profile"\t2025-10-07 11:28:38.507919190 +0200
@@ -20,8 +20,2 @@
dmidecode -s bios-version 0
-dmidecode -s system-manufacturer 0
-dmidecode -s system-product-name 0
-dmidecode -s baseboard-product-name 0
-dmidecode -s processor-version 0
-dmidecode -s bios-vendor 0
-dmidecode -s bios-version 0
flashrom -p internal --flash-name 0
@@ -50 +44,2 @@
reboot 0
+dmidecode 0OSFV test results
|
Signed-off-by: Michał Iwanicki <michal.iwanicki@3mdeb.com>
30 tasks
DaniilKl
reviewed
Nov 6, 2025
Comment on lines
+1957
to
+1977
| set_global_state() { | ||
| local var="$1" | ||
| local value="$2" | ||
| ( | ||
| flock -x 200 | ||
| touch "${DTS_STATE}" | ||
| sed -i "/^${var}=/d" "${DTS_STATE}" | ||
| echo "${var}=${value}" >>"${DTS_STATE}" | ||
| ) 200>"${DTS_STATE_LOCKFILE}" | ||
| } | ||
|
|
||
| get_global_state() { | ||
| local var="$1" | ||
| ( | ||
| flock -x 200 | ||
| touch "${DTS_STATE}" | ||
| # print <val> in first occurrence of <var>=<val> | ||
| awk -F '=' -v var="${var}" '$0 ~ "^" var "=" {print $2; exit}' "${DTS_STATE}" | ||
|
|
||
| ) 200>"${DTS_STATE_LOCKFILE}" | ||
| } |
Contributor
There was a problem hiding this comment.
Can't we dynamically determine free file descriptor and use it? We must not hardcode anything.
Contributor
There was a problem hiding this comment.
We create it ourselves, so why not? What's the possible problematic scenario here?
Contributor
There was a problem hiding this comment.
What's the possible problematic scenario here?
Race condition with other application for the hardcoded file descriptor.
Contributor
Author
There was a problem hiding this comment.
Could be done in e.g. dts-boot but then we will have to keep separate state file per dts-boot (or at least per tty) so there is no race condition (which we might want to do anyway). This could probably be implemented better, as I think it won't work with multiline variables. I just needed some way to change variables from callbacks and this was quick workaround.
| SHOW_FUSE="true" | ||
| else | ||
| SHOW_TRANSITION="false" | ||
| SHOW_FALSE="false" |
Contributor
There was a problem hiding this comment.
A typo?
Suggested change
| SHOW_FALSE="false" | |
| SHOW_FUSE="false" |
Comment on lines
-14
to
-16
| trap : 2 | ||
| trap : 3 | ||
| trap wait_for_input EXIT |
Contributor
There was a problem hiding this comment.
Why have you deleted the traps?
Contributor
Author
There was a problem hiding this comment.
tui-sh already traps 2 of those (except SIGQUIT (3) but I never used this one anyway. It traps SIGTERM instead):
https://github.com/3mdeb/tui-sh/blob/24e778398c4fdc2c6be06ec271a1bb6737a426d2/lib/tui-lib.sh#L46
| trap : 3 | ||
| trap wait_for_input EXIT | ||
| # those won't change | ||
| DTS_VERSION=$(grep "VERSION_ID" ${OS_VERSION_FILE} | cut -d "=" -f 2-) |
Contributor
There was a problem hiding this comment.
Why cant't wwe do it in $DTS_ENV file?
| else | ||
| DPP_KEYS_LABEL="Load your DPP keys" | ||
| fi | ||
| if systemctl is-active sshd &>/dev/null; then |
Contributor
There was a problem hiding this comment.
We should add a function for systemctl is-active sshd &>/dev/null, as we are using this several times in dts-scripts.
Comment on lines
+5
to
+7
|
|
||
| if systemctl is-active sshd &>/dev/null; then | ||
| tui_echo_green "Turning off the SSH server..." |
Contributor
There was a problem hiding this comment.
Are we not going to wait for network connection as was done before:
dts-scripts/include/dts-functions.sh
Lines 1485 to 1488 in 508faf9
?
Contributor
Author
There was a problem hiding this comment.
I'm not sure how good of a design this is. We might want to enable ssh even if there is no network connection (e.g. on local network only). I guess waiting for dhcp to assign IP might be good idea but I think it should already be assigned when we boot into DTS menu
Contributor
There was a problem hiding this comment.
Entire file should be renamed to outline that we toggle the credentials display option.
Contributor
There was a problem hiding this comment.
We need to add more comments. It is hard to review this piece of code.
Contributor
There was a problem hiding this comment.
We should instead focus on fixing the extensions part logic in the next step:
It may be even a dead code in the meantime, as we do not use it at present.
| while IFS=$'\t' read -r file_name file_position _; do | ||
| export file_name file_position callback | ||
| callback="${DPP_PACKAGES_SCRIPTS_PATH}/${file_name}" | ||
| yq -i '.menu += {"key": strenv(file_position), "label": strenv(file_name), "callback": strenv(callback)}' "$extensions_yaml" |
Contributor
There was a problem hiding this comment.
The label should not be file_name, but should be defined by the code inside file_name that is actually a script.
Contributor
Author
There was a problem hiding this comment.
Yes, it should but it'll require changes in extension scripts (so they return their label instead of trying to draw whole menu section by themselves). This is quick workaround to display anything at all.
| export file_name file_position callback | ||
| callback="${DPP_PACKAGES_SCRIPTS_PATH}/${file_name}" | ||
| yq -i '.menu += {"key": strenv(file_position), "label": strenv(file_name), "callback": strenv(callback)}' "$extensions_yaml" | ||
| done < <(yq -p=json '.[] | [.file_name, .file_menu_position] | @tsv' "${DPP_SUBMENU_JSON}") |
Contributor
There was a problem hiding this comment.
Lets be more consistent and use file_menu_position everywhere instead of file_position.
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
4 participants
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.
Related: 3mdeb/tui-sh#1
for simplicity I copied
tui-lib.shhere.Left is current DTS, right is DTS with this PR.
Screencast.From.2025-10-24.14-49-20.mp4