Closed wh201906 closed 9 months ago
You are welcome to add an entry to the CHANGELOG.md as well
@henrygab @iceman1001 Could you please review this PR?
Gentle reminder that we use calloc instead.
Some external libs still uses malloc which we don't want to fiddle with.
I replaced declarations like this
uint8_t bits[XXX]
-> malloc()
uint8_t bits[XXX] = {0}
-> calloc()
Plus, all of the memory allocated by malloc()
is actually initialized immediately by getFromGraphBuf()
in commit 8b6a274e289cd96f16616de741f0d02fb10d1e6e, so I think it's not necessary to zero-initialize them first.
Anyway, it's up to you. Would you prefer to still use calloc()
for them?
I found that using calloc has helped us on different platforms, so yes, that is the prefered way.
I've replaced them
Since I travelling, I will not be able to look at this PR until I get back.
No problem, have a good time!
Got time to play little with this PR today.
looking good, it has some nice potential.
I found that the lf read -r
didn't quite make sense. Should be smoother. end user shouldn't need to know, it should just work.
The demodulation code lfdemod.c
has 512 bit limits, which now can be increased much more.
I get some freezes, indicating there are some memory issues still.
but I like it.
scrolling in the plot window is confusing when its this large..
I found that the
lf read -r
didn't quite make sense. Should be smoother.
But the real-time stuffs has speed requirements. It's not always working, especially for the wireless connections.
I get some freezes, indicating there are some memory issues still.
You mean it's in lfdemod.c
?
I would say it should be automatic..
Default sample size or less, should go the old way, if a larger sample size, it should go the alternative way.
lf read
isn't like real time sniffing..
Hold on with changing demodulation functions. Otherwise this PR will be become too large. Better to change that after we get this one merged
OK. Is there anything I need to do for improving this PR?
Default sample size or less, should go the old way, if a larger sample size, it should go the alternative way.
Sounds reasonable
Check for memory leaks.
Any steps to reproduce the memory leak bugs?
do different lf / data operations?
I got a segmentation fault when running data modulation
over 132000 samples.
It should be fixed now.
There are not too many changes to the demodulation functions, so I included them in this PR.
alright, doing some more testing, this text should be adapted to match new samples.
"data samples -n 10000"
static int CmdSamples(const char *Cmd) {
CLIParserContext *ctx;
CLIParserInit(&ctx, "data samples",
"Get raw samples for graph window (GraphBuffer) from device.\n"
"If 0, then get whole big buffer from device.",
"data samples\n"
"data samples -n 10000"
);
void *argtable[] = {
arg_param_begin,
arg_int0("n", NULL, "<dec>", "num of samples (512 - 40000)"),
arg_lit0("v", "verbose", "verbose output"),
arg_param_end
};
Or...
data samples
is a old style...
The lf search only uses the old one too...
lf search -u
I'm not sure what I should do now...
It looks like the data samples
is only for fetching existing data from the device memory, so I guess it has nothing to do with the real-time samples.
As for lf search -u
, I just tested lf search -1u
to use the samples from the graphbuffer(300000 samples), and it doesn't crash.
One of these days, we should refactor lf search
to read once and then send through all the demodulators...
Making sure we don't do multiple reads
Lets merge,
and see what kind of bugs is coming out.
I have some suggestions for improvements as stated above.
Thank you @iceman1001 and @henrygab for reviewing this PR and #2167
2167 with clear commit history and some minor improvements
Known limitations:
bits_per_sample = 8
is supported now.When LF image is not loaded into the FPGA, the first 14 bytes could be the response of CMD_WTX rather than real samples.(Fixed in #2193)