Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NRF52 debug tools update #297

Merged
merged 2 commits into from
Jun 22, 2023
Merged

NRF52 debug tools update #297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
730b69b
Specify bootloader TTY device by name
szszszsz Jun 21, 2023
bd71660
Add script for remaining stack memory graphing
szszszsz Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion utils/nrf-bootloader/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ UPLOAD_SH = $(BL_DIR)/upload.sh

FW_SIGNED = fw_signed_update.zip
FW_RAW = fw_unsigned.hex
TTYDEV = /dev/ttyACM1
TTYDEV := $(wildcard /dev/serial/by-id/usb-Nitrokey_Nitrokey_3_Bootloader*)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if there are multiple devices plugged?

Copy link
Member Author

@szszszsz szszszsz Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both will be supplied as arguments probably (so the flashing should fail?). I did not consider this to be a use case, but perhaps you could provide the TTYDEV instead while calling Make, if you have multiple devices.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it that case it should bail. Having multiple devices plugged while flashing is likely an error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I do not want to support that. It's out of scope for me. What's the use case for having multiple bootloaders connected?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having multiple bootloaders connected is most likely an error and should abort.
It would be a nice to have safety feature.

Copy link
Member Author

@szszszsz szszszsz Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the motivation, but I do not see how that can happen in the daily use, hence it looks like imaginary case for me, wasting time needlessly. As in, we do not check for free RAM memory or disk space either.
Can we fix that in another PR? Alternatively, I will be fine with a suggestion.
I do not think it can be done in one line (unless switching back to shell with some tee and pipes), so the count will have to be done separately on the actual use.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could easily happen by mistake. Previously worst case scenario it would only flash one, now it would run a command with multiple unexpected arguments, so at least we should be aware of the consequence.

Another option would be to take only the first result of the wildcard with $(world 1, $(TTYDEV)): https://stackoverflow.com/questions/39674277/random-access-arrays-in-makefile

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should abort, or otherwise we are not fixing the main problem this patch was meant to fix (to have a properly selected target device).

Copy link
Collaborator

@sosthene-nitrokey sosthene-nitrokey Jun 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nrf-builder does it this way:

TTY := $(shell ls -1rt /dev/ttyACM* | tail -n 1 | xargs)

The other solution for me would be to use

TTYDEV := $(wildcard /dev/serial/by-id/usb-Nitrokey_Nitrokey_3_Bootloader*)

in nrf-builder

Actually nrf-bootloader/upload.sh already fail if it has more than 2 arguments, so the answer to my initial question was "it falis" which is a satisfying answer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yes, basically it selects the last ttyACM device, assuming alphabetic order, which does not always mean the last connected device. And we still do not know, to what we have connected to. It should fail with multiple devices ideally, as you said. Let's keep this in mind for the next update.
  2. Fails right now, but it may accept more arguments in the future :-) The solution is probably to quote the result, so when multiple paths will be returned by wildcard, it won't be a correct path when parsed as a single argument.

# /dev/serial/by-id/usb-Nitrokey_Nitrokey_3-if02
# /dev/serial/by-id/usb-Nitrokey_Nitrokey_3_Bootloader_F95AF80A9E98-if00

fw-assemble-dfu: $(FW_SIGNED)

Expand Down
2 changes: 1 addition & 1 deletion utils/nrf-builder/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ ALL_ARTIFACTS := $(FW_NAME_RELEASE) $(FW_NAME_PROVISIONER) $(FW_NAME_DEVELOP) \

SRCS = $(shell find $(FW_RUNNER)/src -name "*.rs" )

TTY := $(shell ls -1rt /dev/ttyACM* | tail -n 1 | xargs)
TTY := $(wildcard /dev/serial/by-id/usb-Nitrokey_Nitrokey_3_Bootloader*)

EXTRA_FEATURES :=

Expand Down
1 change: 1 addition & 0 deletions utils/nrf-debugging/binary.elf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../runners/embedded/artifacts/runner-nrf52-bootloader-nk3am.elf
25 changes: 25 additions & 0 deletions utils/nrf-debugging/gdb-stack.cmds
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
set pagination off
set logging file gdb.txt
set logging on

set architecture arm
set endian little
set arm fallback-mode thumb
set arm force-mode thumb
set style enabled off
file binary.elf
target remote :3333


define stepinf
while(1)
p $msp
step
end
end

# Automatically start tracing upon reaching the following line
# b transport.rs:181
# commands
# stepinf
# end
36 changes: 36 additions & 0 deletions utils/nrf-debugging/graph_stack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from matplotlib import pyplot as plt

"""
Quickly graph gdb msp prints.

rg -F "= (*mut ()) " gdb.txt > gdb.txt.stack

Expected format is like:

...
$38057 = (*mut ()) 0x2001b478
$38058 = (*mut ()) 0x2001b478
...

"""

PATH = 'gdb.txt.stack'

def main():
data = []
with open(PATH, 'r') as f:
for line in f:
num = int(line.split()[-1], 16)
num = num - 0x20000000
num = num // 1024
data.append(num)

print(len(data))
plt.plot(data)
plt.ylabel('free stack memory left [kB]')
plt.xlabel('sample no.')
plt.show()


if __name__ == '__main__':
main()