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

ISO15693 codec and applications updates #274

Merged
merged 7 commits into from
Sep 11, 2020

Conversation

ceres-c
Copy link
Contributor

@ceres-c ceres-c commented Sep 4, 2020

This PR includes and supersedes #272

Me and @MrMoDDoM have been refining ISO15 code due to an issue we faced with the ST CR95HF reader. We thought something was wrong with the codec because the Read Multiple Blocks failed when reading more than 7 blocks at once. It felt like an issue when modulating long frames but, after some investigation it turned out that the ST reader was very precise with SOF timings.
Reading more than 7 blocks into the codec's buffer take too much time, and this means that ISO t1 time elapses before data is ready and the codec is stuck waiting for data before it can modulate anything, so the reader complains as it did not receive a SOF in time. This kind of issues is not present with the phones we've tested the code with

I'd like to discuss about ISO t1 time, since we could not explain how the old value 4192 + 128 + 128 - 1 was chosen. This value is greater than both ISO dictated nominal and maximum t1, but yielded better results than t1 nominal, so reading 10 blocks at a time was possible. This is possibly due to the fact that waiting "a bit more" allows the Application to generate needed data before the first interrupt hit occurs and, at that time, data is marked as ready to be processed by CodecTask.
How was the old value chosen? Do we want to favor performance over emulation accuracy? Which value should be kept?

Also, applications were updated to improve performance. Reading the UID on every ApplicationTask hit was totally unnecessary and a big performance drag

Since GCC 10, the compiler defaults to -fno-common, thus variables with
multiple tentative definitions result in linker errors.
We've (me and @MrMoDDoM) extern-ed the shared variables to fix the issue.

Also, when I implemented ISR sharing, somehow I forgot about a function
which was shared as well, so I fixed that mistake and added a couple of
comments on how shared function calls work.
Unvarying interrupt configuration moved from StartISO15693Demod to CodecInit
Fixed an old issue (geo-rg#4) properly
overwriting the PERBUF register when no data has to be sent
Loadmodulation ISR has a new GOTO label which is called when ISO t1 time is
elapsed, but data is not yet ready. This way it is possible to avoid checking
for state validity on every ISR hit
New inline function to clean up interrupts when field noise (garbage)
is incorrectly picked up
Bitrate configuration in ISO15693_EOC has been simplified

Cleaned up header file with useless defines. Moved defines to .c when there was
no need to share them with other source files

Annotated numer of clock cycles consumed by asm ISR sharing routine

As usual, this work was done with @MrMoDDoM
Removed the if-elseif in EM4233 to use a switch case which yields minor
speed improvements of ~1,5 microseconds
Avoid reading the UID every AppProcess tick in both EM4233 and TiTagIT
to save ~11 microseconds.
Copy link
Collaborator

@fptrs fptrs left a comment

Choose a reason for hiding this comment

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

My compiler throws this error:

Application/TITagitstandard.c:220:17: error: ‘NewUid’ undeclared (first use in this function)
     memcpy(Uid, NewUid, ActiveConfiguration.UidSize); // Update the local variable

@ceres-c
Copy link
Contributor Author

ceres-c commented Sep 9, 2020

Very sorry about that, it was a rogue copy-paste. I'll fix it tomorrow :)
BTW I have no idea how it did slip through, since I was pretty sure I tested everything

@ceres-c
Copy link
Contributor Author

ceres-c commented Sep 9, 2020

Done

ISO15693 CRC calculation is slow and it delayed too much transmission of long
frames. Some readers marked transmission as unsuccesfull and ignored data.
Moving CRC calculation inside the codec allows to start modulating the field
while the CRC is not yet ready but, by the time it has to be trasmitted,
calculation will be done.
This fixes emsec#262

In EM4233, there still are issues with Read Multiple command when block
locking status is requested for more than 0x1F blocks

Implemented a check in ISO15693 codec to avoid out of bound CRC writes.
@ceres-c
Copy link
Contributor Author

ceres-c commented Sep 10, 2020

Fixed the long standing #262

@fptrs fptrs merged commit e3af29a into emsec:master Sep 11, 2020
@ceres-c ceres-c deleted the iso15693-codec-upd branch September 11, 2020 21:21
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.

2 participants