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.

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

Real-time LF sampling #2167

Closed
wants to merge 27 commits into from
Closed

Conversation

wh201906
Copy link
Contributor

@wh201906 wh201906 commented Nov 8, 2023

(moved to #2173)

  1. The client side doesn't handle these samples. To get the samples, you need to run socat with -x option to log the data, then plot it somewhere.
  2. When starting bulk tranfer, ~3 samples will lost due to the time cost. Some of the USB transfer payload can be distributed into every sample. (Done in 523fde2)
  3. I use the verbose option as a flag for real-time sampling (-r for real-time sampling)

1. The client side doesn't handle these samples. To get the samples,
you need to run socat with -x option to log the data, then plot it
somewhere.
2. When starting bulk tranfer, ~3 samples will lost due to the time
cost. Some of the USB transfer payload can be distributed into every
sample.
3. I use the verbose option as a flag for real-time sampling
Copy link

github-actions bot commented Nov 8, 2023

You are welcome to add an entry to the CHANGELOG.md as well

@wh201906
Copy link
Contributor Author

wh201906 commented Nov 8, 2023

2. When starting bulk tranfer, ~3 samples will lost due to the time cost

Maybe this is caused by the -x option in socat
Test hw status:

socat -dd /dev/ttyACM0,raw,echo=0,b115200 udp-listen:18888,fork,reuseaddr
Transfer Speed PM3 -> Client... 732160 bytes/s

socat -dd -x /dev/ttyACM0,raw,echo=0,b115200 udp-listen:18888,fork,reuseaddr
Transfer Speed PM3 -> Client... 325354 bytes/s

stdbuf -oL socat -dd -x /dev/ttyACM0,raw,echo=0,b115200 udp-listen:18888,fork,reuseaddr &> lfread3.txt
Transfer Speed PM3 -> Client... 304956 bytes/s

# Can be optimized by this
stdbuf -oL socat -dd -x /dev/ttyACM0,raw,echo=0,b115200 udp-listen:18888,fork,reuseaddr &> /mnt/ramdisk/lfread3.txt
Transfer Speed PM3 -> Client... 675840 bytes/s

@iceman1001
Copy link
Collaborator

Interesting
However, all those USB stuff doesn't belong in the lfsampling...
you need to refactor it

@wh201906
Copy link
Contributor Author

wh201906 commented Nov 9, 2023

Yes I will. The commits there are only for test now. I'll clean up the code and squash the commits once everything is done.

@iceman1001
Copy link
Collaborator

I am happy to see your great efforts of making the proxmark3 run better with new functions.

Copy link
Contributor

@henrygab henrygab left a comment

Choose a reason for hiding this comment

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

First, great job! This looks like a clean method of using the hardware FIFO with a single collection buffer.

I do have a number of recommended changes. Some are defensive coding, which are recommended because this is a larger, multi-author project. Some are minor formatting nitpicks.

The two new usb function names ... that's a bigger discussion. I stumbled over what those functions were doing initially, and even now am not 100% sure I get it.

Finally, I could use help understanding why the decimation setting should alter the maximum bytes per USB transfer (details in discord).

Overall, I can't wait for this feature to hit release!

armsrc/lfsampling.c Outdated Show resolved Hide resolved
armsrc/appmain.c Outdated Show resolved Hide resolved
armsrc/appmain.c Outdated Show resolved Hide resolved
armsrc/appmain.c Show resolved Hide resolved
armsrc/appmain.c Outdated Show resolved Hide resolved
client/src/cmdlf.c Outdated Show resolved Hide resolved
client/src/cmdlf.h Outdated Show resolved Hide resolved
common_arm/usb_cdc.c Outdated Show resolved Hide resolved
armsrc/lfsampling.c Show resolved Hide resolved
common_arm/usb_cdc.h Outdated Show resolved Hide resolved
@wh201906
Copy link
Contributor Author

In cast that I miss the review advices you provided, I'll apply all of them first and rewrite the commit history as need.

@wh201906
Copy link
Contributor Author

@iceman1001 Do you consider using the unlikely() GCC extension? The arm code is always compiled with gcc-arm-none-eabi on all platforms. If you don't like it I will revert the changes about them.

@wh201906
Copy link
Contributor Author

wh201906 commented Nov 11, 2023

Well it looks like the unlikely() is undefined. I tested it with Compile Explorer and it shows it's undefined, just like the CI output.
However, the __builtin_expect() does work, so we need to add an extra #define for it if we need unlikely()

@iceman1001
Copy link
Collaborator

I seriously doubt that the __builtin_expect() call will improve the loop much.

and since we compile it on decently recent versions on the gcc-arm-none-eabi compilers (2018-2022 ish) across the OS platforms, we can add it.

I would love to see if there is actually some gains, otherwise these hints adds more confusing when trying to read the code.

@wh201906
Copy link
Contributor Author

I see

common_arm/usb_cdc.c Outdated Show resolved Hide resolved
@wh201906
Copy link
Contributor Author

@iceman1001 The samples can be fetched to the client now.

samples = WaitForRawDataTimeout(realtimeBuf, samples, 1000, true);
// ...
free(realtimeBuf);

The real-time sampling process can be stopped by pressing Enter on the client or pressing the button on PM3. It can also be stopped if it reaches the number of -s option.
How can I store them into a file compatible with the .pm3 format? Is there any existing function for this?

@wh201906
Copy link
Contributor Author

@iceman1001 The real-time lf read is working now. However, I added some of the code from cmddata.c to cmdlf.c to make it work. Plus, the graph buffer is not resizable, so it's hard to plot and save all of the samples. I think we need to refactor the code.

client/src/cmdlf.c Fixed Show fixed Hide fixed
@iceman1001
Copy link
Collaborator

Nice!

data save -f is the function all. it save the length of what has been loaded onto the graphbuffer.

@wh201906
Copy link
Contributor Author

wh201906 commented Nov 12, 2023

Here are some bugs when bit_per_sample is not 8

  1. async_usb_write_requestWrite() always returns false
  2. the client crashes after receiving the data should be fixed now

@wh201906 wh201906 mentioned this pull request Nov 14, 2023
@wh201906
Copy link
Contributor Author

I've cleaned up the commit history and made a new PR #2173.

@wh201906 wh201906 closed this Nov 14, 2023
@wh201906 wh201906 deleted the lf_sniff branch December 1, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants