iceman1001 / ChameleonMini-rebooted

Chameleon Mini revE rebooted - Iceman Fork, the ChameleonMini is a versatile contactless smartcard emulator (NFC/RFID)
Other
396 stars 85 forks source link

Fixed config id managemt #205

Closed nitraiolo closed 2 years ago

nitraiolo commented 2 years ago

Fixed config id management so now it's possible to use the 0-7 interval like on Rev. G

nitraiolo commented 2 years ago

I've opened the PR on first push but all subsequent went in the same PR. Sorry, I don't know how to split them then...

iceman1001 commented 2 years ago

I know, it happens. All commits goes into your PR. Normally you make a branch and make a PR from it instead.
Now we take it bit by bit instead.

nitraiolo commented 2 years ago

Next time I'll do a branch then, Thanks! About the fixes I've tested them with MCT comparing dump of real cards with and without keys and in a real environment.

MageDelfador commented 2 years ago

Good job. I'm glad to see my name. I don't always use sprintf() because sprintf() takes more time then direct assignment, but it doesn't seem important. MFCLASSIC_LOG_MEM_LINE_BUFFER_LEN 256 is still too few for logs.Writing logs to flash takes several milliseconds of CPU time. So we can only write all logs to buffer during communication. Otherwise, an error will occur. It really needs a big buffer.

nitraiolo commented 2 years ago

Thanks @MageDelfador . To avoid errors, I'm using two 256 byte buffers swapped at write time, so even write should be faster with a smaller buffer. At the moment it seems enough in my tests. Can you test also this in your environment too? I'm really curious.

MageDelfador commented 2 years ago

OK, I'll test it later. I just read the code again, and find some problems.

/ swap buffers / if ( LogLineBufferFirst ) { LogLineBuffer = LogLineBufferA; LogLineBufferFirst = true; } else { LogLineBuffer = LogLineBufferB; LogLineBufferFirst = false; }` There seems to be a logical bug here.

And you can read Firmware/ChameleonMini/Memory/SPIFlash.c line 188 It is a software SPI and no multithreading. So we can't do other work while writing flash. When there are many logs, the extra logs will be lost.

nitraiolo commented 2 years ago

You have right about condition... this is the result of the merge of two code bases in which I used two different conditions... sorry

MageDelfador commented 2 years ago

That's not the point, You should understand that you can't write the log to the buffer while writing to flash. AppWorkingMemoryWrite will blocking until writing done.

nitraiolo commented 2 years ago

I don't understand why a bigger buffer can solve the problem if log is written on Tick basis .ApplicationTickFunc = MifareClassicAppLogWriteLines, every 100ms in the main loop then. Bigger buffer = bigger time to write. You don't have the control over the time when it is written. A possible solution then could be using a bigger buffer, as you advise, but protecting it from frequent write using a condition to write it like if ( BUFFER_PERCENTAGE > XX% and BUFFER_MAX_TICK_COUNTER <= LIMIT_COUNT) in the MifareClassicAppLogWriteLines function.

MageDelfador commented 2 years ago

Big buffer can get a complete log. Because the time of reading card is short, and all of log is producted at this time.If writing flash during this time, it will cause card reading time out error. In order to get a complete log, you need to write the log to the big buffer first, and then write it to flash at another time. As long as the flash is not written at the time of card reading, a longer writing time is allowed.

MageDelfador commented 2 years ago

No matter how short the written data is, writing flash during card reading will cause card timeout error.Two buffers do not solve any problems.

nitraiolo commented 2 years ago

@MageDelfador I've done a branch for this experiments in my fork: https://github.com/nitraiolo/ChameleonMini-rebooted/tree/bigbuffer we can continue this discussion there.