binascii.crc32 and/or binascii.crc_hqx (=crc16)

[bludin]

I would like to implement a file-transfer over serial protocol and to that end I need to calculate the CRC of typically 1kB byte-packets. Unfortunately, the latter is pretty slow in Python and CPython's binascii.crc32() and binascii.crc_hqx() are missing from circuitpython (because they use the implementation from zlib which is not turned on because of size and instability, according to Dan). Would anybody be willing to implement them directly in circuitypython's binascii?? It should only be relatively few lines of code, but the procedure is (still) beyond my skills to attempt it myself. BTW, the SAMD51 apparently has a CRC32 engine. Maybe that could be leveraged for this platform?

Cheers, beat

Edit: I just found this https://github.com/adafruit/circuitpython/blob/main/lib/uzlib/crc32.c . Sorry for being ignorant, but does this mean it's already present or could be simply activated and if so, how?

[tannewt]

It looks like there is code to hook crc32 into binascii: https://github.com/adafruit/circuitpython/blob/main/extmod/modubinascii.c#L207-L219 The setting is MICROPY_PY_UBINASCII_CRC32.

We have it turned off. It could be enabled on full builds like the other settings here: https://github.com/adafruit/circuitpython/blob/main/py/circuitpy_mpconfig.h#L209-L236

[bludin]

thanks a lot for your help :) so I've inserted 3 lines in _circuitpy_mpconfig.h#L238

#ifndef MICROPY_PY_UBINASCII_CRC32 #define MICROPY_PY_UBINASCII_CRC32 (CIRCUITPY_FULL_BUILD) #endif

but when I do a clean build, I get the following error:

bludin:~/circuitpython/ports/atmel-samd$ make BOARD=pybadge -j6 Use make V=1, make V=2 or set BUILD_VERBOSE similarly in your environment to increase build verbosity. mkdir -p build-pybadge/genhdr FREEZE ../../frozen/circuitpython-stage/pybadge GEN build-pybadge/genhdr/moduledefs.h QSTR updated /home/bludin/bin/gcc-arm-none-eabi-10-2020-q4-major/bin/../lib/gcc/arm-none-eabi/10.2.1/../../../../arm-none-eabi/bin/ld: firmware.elf.lto.o: in function 'mod_binascii_crc32': <artificial>:(.text.mod_binascii_crc32+0x2a): undefined reference to 'uzlib_crc32' collect2: error: ld returned 1 exit status make: *** [Makefile:390: build-pybadge/firmware.elf] Error 1

I guess the problem is the undefined reference to uzlib_crc32 but modubinascii.c contains #include "lib/uzlib/tinf.h" which contains #include "uzlib.h" which contains uint32_t TINFCC uzlib_crc32(const void *data, unsigned int length, uint32_t crc);

but that's as far as I get with my ignorance of c ...

[bludin]

I've also tried to move the crc32 code from uzlib directly into binascii (not sure if that's the way to go but I tried anyway)

https://github.com/Life-Imaging-Services/circuitpython/blob/crc32/extmod/modubinascii.c#L208-L279

After prefixing L255 with static, circuitpython builds w/o error. But it doesn't boot and I'm lightyears out of my waters here. Any help is greatly appreciated :)

[tannewt]

You are making progress! The undefined reference error is coming from the linker when it's looking for the implementation. This happens when the source file hasn't been listed for CircuitPython to build. Try listing the source file in this list: https://github.com/adafruit/circuitpython/blob/main/py/circuitpy_defns.mk#L686

[dhalbert]

I've also tried to move the crc32 code from uzlib directly into binascii (not sure if that's the way to go but I tried anyway)

I think that's a good way to go, because then we are not dragging in uzlib and its complications. I would then remove the MICROPY_PY_UBINASCII_CRC32 flag use, and just make it included all the time. It's quite small and we can spare the space on boards that already are including binascii.

[bludin]

I think that's a good way to go

ok, I removed the flag use, but I still have the same problem, i.e. circuitpython builds w/o errors but when I load the firmware.uf2 on a device (e.g. a Pyportal Titano) it no longer boots.

Any idea what I might be doing wrong? (Does the capitalization of STATIC vs static matter in C?) https://github.com/Life-Imaging-Services/circuitpython/blob/crc32/extmod/modubinascii.c#L208-L289

[dhalbert]

The capitalization doesn't matter. If you do a build without your changes (just the tip of main), does it work? If that doesn't work, then there's a general build issue. Is there any error printed by the build?

[bludin]

If you do a build without your changes (just the tip of main), does it work?

No, it doesn't anymore, not from the tip of 'main'. I've been successfully building until recently, though. 7.1.0 was the last functional build that booted. So I checked out that tag and, low and behold, got a successfully booting firmware.uf2. Then I made a branch from there, modified modubinascii.c as before and started getting build errors related to ulab(?). I checked out 7.1.0 again (which should erase any changes, as far as I understand git) and the build errors started to multiply. Suspecting that I screwed up somewhere with git, I've started over with a fresh fork. This got me back to square one, i.e. circuitpython builds without errors, but the Pyportal won't boot. Scratching my head right now...

[dhalbert]

If you forked circuitpython, make sure your own main branch does not have spurious commits on it. It should be the same as the upstream repo's main. Assuming that is the case, bring your main up to date, do a make fetch-submodules, and then try building it. Then do checkout -b the_pr_branch_name, make your changes, and try the build again.

I think your own repo (local or fork) has somehow gotten out of sync, possibly just due to submodule version skew.

[tannewt]

@bludin we're happy to help on Discord too in #circuitpython-dev: https://adafru.it/discord

I suggest using git status to double check that your git state is what you expect.

[bludin]

sorry for being unclear - I freshly forked from adafruit/circuitpython today, i.e. from scratch. To exclude the possibility that I the problem was because I got out of sync. So the problem must lie elsewhere, but I can't imagine where, right now.

[bludin]

ok, bulids work again. Problem was most likely related to #5951

[bludin]

... successfully did the changes discussed and opened my very first PR ever

[dhalbert]

Fixed by #5969. Thanks!