added DigitalSequence and PulseReader

[g3gg0]

added DigitalSequence to chain multiple DigitalSignals
added PulseReader for hardware assisted digital signal sampling

What's new

• adds DigitalSequence to queue multiple DigitalSignal in a scripted manner to reducre precalculation effort
• adds PulseReader to sample GPIOs whenever their state change and capture TIM2 value along with it

Verification

Maybe with such a test code? Where to put smth like that?

    /* read pulses from PB2 */
    PulseReader *reader_signal = pulse_reader_alloc(&gpio_ext_pb2, 512);
    pulse_reader_set_timebase(reader_signal, PulseReaderUnitNanosecond);
    pulse_reader_start(reader_signal);
    while(true) {
        uint32_t length= pulse_reader_receive(reader_signal, timeout_ms * 1000);
        FURI_LOG_D(TAG, "Pulse: %lu", length);
    }
    pulse_reader_stop(reader_signal);

Checklist (For Reviewer)

• PR has description of feature/bug or link to Confluence/Jira task
• Description contains actions to verify feature/bugfix
• I've built this code, uploaded it to the device and verified feature/bugfix

[g3gg0]

I know this looks a bit low level.
We also can use a C-only version but then minimum length of a the last pulse in a signal has to be a bit longer.

[skotopes]

I don't think that C version will be slower here. This code is quite straight forward, no special instructions used. Please provide C-version.

[g3gg0]

Also thought about using DMAs to automate things, but that would probably occupy all available DMA channels of both DMA.
And even then I am not sure if it would work out.

[g3gg0]

ouch. i guess i know what to do...

[gornekich]

Hi @g3gg0
Could you please also commit firmware/targets/f7/api_symbols.csv file. You can replace '?' field with '-' for now

[g3gg0]

Ah cool, it seems you are doing PIL tests with a flipper attached?

Currently I am trying to get the unit test setup running on my setup.
I think I will have it ready in a few minutes, but is there some documentation / hints about this topic?

EDIT: dumb question: how to get the debug log in unit_test mode?

[g3gg0]

Ok, was due to signal->gpio being uninitialized, where the prepare code used it to configure the GPIO mask.
That should be updated in the unit_test file to also check signal->gpio_buff to be properly initalized.

Then we are left with a difference in the timings, which i have to check. Maybe that's due to the trailing level (?)

As I said, I have to look closer.

[gornekich]

Hello @g3gg0
Thanks for this PR! I can also see that nfc_digital_signal_test unit test fails. Could you have a look on it?
Some hints regarding unit tests: you can build firmware with unit tests included via ./fbt FIRMWARE_APP_SET=unit_tests More details you can find in documentation/fbt.md
The default way to read logs is to connect Flipper GPIO 13 (TX) to uart-usb converter. The UART baudrate is 230400

[g3gg0]

okay, took me some days to get back to work on it.
yes, it is due to the trailing level.

Let's get more into detail
NfcA code is "asking" to generate pulses of a specific length by concatenating them using digital_signal_append().
when normalizing the µs values to the smallest (NfcA) pulse length for readability, we get those patterns:

"one" is the one-bit, "zero" the zero-bit. both consist of 1 long period and 8 short pulses, changing modulation strength.
Due to polarities, for the "one", one short and the long period merge:

In our discussion nov 21 on telegram, we talked about two interpretations of the code:

vs

My first proposal was A, giving just the guarantee that the pulse will be at minimum duration d.
You said the latter (B) is the one you expect, which I agree, because it feels more natural if you use the code to generate pulses.
Just when appending signals it might confuse users a bit more than variant A as you might want to explain: "digital_signal_append() will extend the last pulse when the signal to append starts with the same level"

However the test data seems to show a whole different interpretation, as the last zero bit has only 7 pulses.
So it neither does a trailing polarity change as in B, nor it would guarantee a minimum pulse length like in A.
It would just check for generating a polarity change without respecting the edge duration.

So I have some questions:
a) is there some specification on which the test is based on, or was it based on the first (reference) implementation
b) shall we change the test to expect "...641 36 37 37 37 37 36 37 37" to match variant B?
c) or am I missing something? :)

[skotopes]

@gornekich @g3gg0 un-draft when ready. ;-)

[g3gg0]

@gornekich

shall i change the test files to match either A or (preferrably) B ?

[g3gg0]

okay i changed the status back to "open" for a review.

• I have changed the test case to match the expected behavior
• not sure about the API version i should have picked

[gornekich]

Hi @g3gg0
Thanks for changes. Very sorry for late responses, I will test PR today

[g3gg0]

no worries.
is there anything i can help with - demo apps or so?

[gornekich]

Great work!
Digital Signal and Sequence work just fine.
While testing PulseReader I noticed strange behavior. If I run the code you suggested for testing and generate 1 Hz square wave with 50% duty cycle, I receive these logs:

25207 [D][PulseReader] Pulse: 131
26203 [D][PulseReader] Pulse: 999854
26205 [D][PulseReader] Pulse: 132
27202 [D][PulseReader] Pulse: 999854
27204 [D][PulseReader] Pulse: 134
28201 [D][PulseReader] Pulse: 999852
28203 [D][PulseReader] Pulse: 153
29200 [D][PulseReader] Pulse: 999833
29201 [D][PulseReader] Pulse: 124
30199 [D][PulseReader] Pulse: 999862
30200 [D][PulseReader] Pulse: 129
The timebase is set to PulseReaderUnitMicrosecond

I expected to see roughly the same values. Am I wrong?

[gornekich]

Also thanks for changing unit tests data. I agree with your solution.

Could you also modify build scripts so that PulseReader is build and installed as static library. You can apply the patch attached
static_lib_patch.txt

[g3gg0]

If I run the code you suggested for testing and generate 1 Hz square wave with 50% duty cycle, I receive these logs:

uh thats weird. looks like
LL_DMAMUX_SetRequestGenPolarity(NULL, LL_DMAMUX_REQ_GEN_0, LL_DMAMUX_REQ_GEN_POL_RISING);
would have been set to LL_DMAMUX_REQ_GEN_POL_RISING_FALLING
the polarity change creates an EXTI, which is a pulse with approximately the duration you have seen IIRC

how did you create the signal? using an external device or with the device creating the signal itself?
guess you are sure the pulses were 50% duty?

will try a similar setup. maybe there is something odd due to timer overruns or such...

[g3gg0]

also: the code checks the polarity everytime you get an pulses's duration

if the polarity didn't change (because the edge was shorter than the EXTI pulse itself), you get PULSE_READER_LOST_EDGE.
so if you are sure you had 50% duty, then i would have said its a fluke in the code, but that check should have triggered and telling you.

[gornekich]

I just took another flipper and generated signal with Signal generator application. I will check with logic analyzer the signal just to be sure.

[g3gg0]

Finding: the pulse_reader's first returned signal was a missing edge. fixed that.

Adding a note that interrupts must not get disabled when using pulse_reader, as it uses EXTI,
which is an interrupt line which would get blocked...

Also added a unit test routine, which I am cleaning up right now.

[g3gg0]

okay, added a unit test, creating different signals using TIM1 that will get read by digital_reader.

for this PA6 and PA7 need to get bridged.
we might have to remove the test anyway, as it doesn't seem to be too stable.
i guess the scheduling kicks in and causes a delay in the code that counts the "sent" pulses and stops the TIM1.
this delay makes the tests of 1-2ms cycles too unstable. 10ms and above seem to work fine.

can you test it if it works for you?

theses the digital pulses that are generated:

[nvx]

Updated f18 api_symbols.csv and rebased on latest dev again. Hopefully the workflows like that better. Will catch up the ISO15693 branch after this is merged prior to un-drafting that PR.

[skotopes]

Un-draft when ready.

[gornekich]

Thank you!
Please consider my suggestions.
Also I attach a patch that suppresses static code analyzer warnings
fixes.txt

[gornekich]

Also I noticed that mf classic emulation works worse than dev brunch. I will try to figure it out

[g3gg0]

Also I noticed that mf classic emulation works worse than dev brunch. I will try to figure it out

wait. you mean that these changes make mf classic emulation worse?

[gornekich]

Yes, but it's partially my fault. I have log level set to debug and message "[prepare] pulse_duration out of range" makes timings worse. When I set debug level to info, the emulation is exactly like in dev brunch.

Now I want to figure out why we fall in if(pulse_duration < 10 || pulse_duration > 10000000) check during mf classic emulation. It looks like we prepare signal->edge_timings not correctly. Sometimes the last value in edge_timings buffer is 0x0a, which cause problems in filling reload_reg_buff

[g3gg0]

at some point in time in my dev branches i moved the NfcA code over to the DigitalSequence stuff.
didn't notice it there - or - accidentally fixed it along with rewriting :D

[g3gg0]

g3gg0@0cb784a
there i have undone my own rewriting
and there is the source:
https://github.com/g3gg0/flipperzero-firmware/blob/c1b5394b0f94d52d2b1867aadacde85db47dc66b/lib/nfc/protocols/nfca.c

[gornekich]

@nvx Sorry for incorrect patch. Please format sources
./fbt format

[gornekich]

@nvx @g3gg0 thanks for the PR and fixes. Please undraft if no changes will be done