Skip to content

Move mbed tls lib to separate folder #6636

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

Conversation

daniel-cesarini-tridonic-com
Copy link

@daniel-cesarini-tridonic-com daniel-cesarini-tridonic-com commented Apr 13, 2018

Description

Move mbedtls feature into separate TARGET_MBEDOS_MBEDTLS in order to exclude it from compilation and clean up the codebase. Also solves some compilation issues for combinations of using / not using proper RNG devices.

As a consequence each device that would use MBEDTLS would need to explicitly enable it in targets.json or in mbed_app.json or in some other way.

This PR is related to ARMmbed/mbed-client#526 .

Pull request type

[ ] Fix
[X] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change


FYI: @MarceloSalazar @karsev

Internal reference tridonic: @markus-becker-tridonic-com

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 13, 2018

As a consequence each device that would use MBEDTLS would need to explicitly enable it in targets.json or in mbed_app.json or in some other way.

Move mbedtls feature into separate TARGET_MBEDOS_MBEDTLS in order to exclude it from compilation

I do not think this is correct. It was deliberately put into that folder to be available as the rest of the modules in mbed OS.

good example is one opened Pr related #6577

I do not fully understand the compilation issues. But would like to hear more about cleaning up the codebase.

@daniel-cesarini-tridonic-com
Copy link
Author

@0xc0170 thank you for the feedback. I will try to improve the solution we are using currently.
Some issues we faced seem to be related to forward porting our application to newer mbed-os versions.

@daniel-cesarini-tridonic-com
Copy link
Author

So in general the approach in mbed is to try to have features available in non "TARGET_" or "FEATURES_" folders if they might apply for multiple targets?
What is the rationale behind it?

Thank you.

@sg-
Copy link
Contributor

sg- commented Apr 19, 2018

Also solves some compilation issues for combinations of using / not using proper RNG devices.

Can you give an example of this. Removing mbed-tls from the default compilation doesn't seem the right way to fix this. In general, the mbed-os is building all software capabilities for a single target by default and we're not looking to conditionally compile OS capabilities at the moment.

@ciarmcom
Copy link
Member

ARM Internal Ref: IOTSSL-2241

@0xc0170
Copy link
Contributor

0xc0170 commented May 3, 2018

We are closing this. Feel free to comment , we would like to understand what issues you were having. we can create a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants