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

Fix undefined type error and ESP32 RTOS timer function call #277

Merged
merged 3 commits into from
Nov 12, 2021

Conversation

rashedtalukder
Copy link
Contributor

There was an issue when using mbedTLS where the wrapper library took arguements of type mbedtls_mpi in atca_mbedtls_ecdsa_sign(), however the the header file where that type is defined was in the source file. This threw a link error.

Also flips the order of the ESP32 timer abstraction. Based on the condition set by the macro, it was looking for the wrong function name.

Checklist

@bryan-hunt
Copy link
Contributor

Commit b7915e5 will produce a conflict when ATCA_USE_RTOS_TIMER is defined which it should be when using FreeRTOS. I assume this was throwing an error because hal_freertos.c was not included in the project?

@rashedtalukder
Copy link
Contributor Author

hal_freertos.c is included. If I flip the order back, this is the error I receive:

/Users/rashed/.espressif/tools/xtensa-esp32-elf/esp-2021r1-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/esp-cryptoauthlib/libesp-cryptoauthlib.a(atca_iface.c.obj):(.literal.atwake+0x0): undefined reference to `atca_delay_ms'
/Users/rashed/.espressif/tools/xtensa-esp32-elf/esp-2021r1-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/esp-cryptoauthlib/libesp-cryptoauthlib.a(atca_iface.c.obj): in function `atwake':
/Users/rashed/test/components/esp-cryptoauthlib/cryptoauthlib/lib/atca_iface.c:248: undefined reference to `atca_delay_ms'
/Users/rashed/.espressif/tools/xtensa-esp32-elf/esp-2021r1-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/esp-cryptoauthlib/libesp-cryptoauthlib.a(atca_iface.c.obj): in function `atidle':
/Users/rashed/test/components/esp-cryptoauthlib/cryptoauthlib/lib/atca_iface.c:278: undefined reference to `atca_delay_ms'
/Users/rashed/.espressif/tools/xtensa-esp32-elf/esp-2021r1-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/esp-cryptoauthlib/libesp-cryptoauthlib.a(calib_execution.c.obj): in function `calib_execute_command':
/Users/rashed/test/components/esp-cryptoauthlib/cryptoauthlib/lib/calib/calib_execution.c:498: undefined reference to `atca_delay_ms'
/Users/rashed/.espressif/tools/xtensa-esp32-elf/esp-2021r1-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: /Users/rashed/test/components/esp-cryptoauthlib/cryptoauthlib/lib/calib/calib_execution.c:512: undefined reference to `atca_delay_ms'

@bryan-hunt
Copy link
Contributor

Actually looking at it further I see that that the function names in this file are incorrect. The reference to ATCA_USE_RTOS_TIMER can be removed and the functions should be atca_delay_us and atca_delay_ms should be hal_delay_us and hal_delay_ms respectively.

@rashedtalukder
Copy link
Contributor Author

rashedtalukder commented Nov 11, 2021

Makes sense. I'll need to send a different PR to Espressif's port then as well. Sorry for not looking deeper and mixing up some of my commits here. I can validate that with these changes it works as expected.

@bryan-hunt
Copy link
Contributor

We haven't added FreeRTOS support to our CMake configuration yet - are you using your own configuration files for this? In our harmony configuration when FreeRTOS is connected in the component grid it then adds the following to the atca_config.h file:

#define atca_delay_ms   hal_rtos_delay_ms
#define atca_delay_us   hal_delay_us

If we add FreeRTOS support to the CMake files so you could directly add the library I think we should probably modify how this is done - for the moment however in your situation adding the above defines should help (but it still would require the changes as you've submitted here as well).

@rashedtalukder
Copy link
Contributor Author

rashedtalukder commented Nov 11, 2021

I'm using the esp-cryptoauthlib project to work with FreeRTOS and have modified their config file.

I need to use their espTLS library (wrapper to mbedTLS), which directly references the esp-cryptoauthlib target. I'm not a CMAKE wizard to know how to override it so it will reference this library instead of theirs (though they have a concept of component aliases). I've also written my own thread-safe HAL since I have multiple i2c devices on the same port.

@rashedtalukder
Copy link
Contributor Author

With this merged in, I can just submodule this repo/commit in to my ported library.

@bryan-hunt
Copy link
Contributor

Thanks for the link to the integration - it makes a lot of sense now that the definitions go flipped in the port. I think there are some improvements we can make in the future but for the moment everything looks good here.

@bryan-hunt bryan-hunt merged commit e320651 into MicrochipTech:main Nov 12, 2021
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.

None yet

2 participants