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

Adds a FreeRTOS-Plus-TCP module. #448

Merged
merged 2 commits into from
Aug 30, 2020

Conversation

mikewolfram
Copy link
Contributor

Fixes part of #447.

@mikewolfram
Copy link
Contributor Author

I also have the require NetworkInterface.c file, but would add that in the PR of the ETH driver.

@mikewolfram
Copy link
Contributor Author

Oh, and I have -Werror=format enabled, which let's the build fail. Not sure if I rather remove it from my build or add a patch to fix the line in FreeRTOS TCP.

ext/aws/FreeRTOSIPConfig.h.in Outdated Show resolved Hide resolved
ext/aws/FreeRTOSIPConfig.h.in Outdated Show resolved Hide resolved
@salkinium
Copy link
Member

Not sure if I rather remove it from my build or add a patch to fix the line in FreeRTOS TCP.

You can add a patch file to the freertos-partial repo and make update.py patch it. I do this for modm-devices too.

@mikewolfram
Copy link
Contributor Author

Working on it I was thinking to add something like

#if defined __has_include
#if __has_include(<FreeRTOSIPConfigLocal.h>)
#include <FreeRTOSIPConfigLocal.h>
#endif
#endif

This would allow the user to add additional defines for FreeRTOS TCP. Or maybe better use a collector in the project.xml file? How would that look like and how to get them into FreeRTOSIPConfig.h?

@mikewolfram
Copy link
Contributor Author

What is needed to get this checks pass?

@rleh
Copy link
Member

rleh commented Aug 25, 2020

What is needed to get this checks pass?

Log: https://circleci.com/api/v1.1/project/github/modm-io/modm/20914/output/103/0?file=true&allocation-id=5f45582fa6592e387c3f81cd-0-build%2F172F83F4

Error generating documentation for device stm32f103c8t6: In Module 'modm:freertos:tcp': File or directory not found!
In file '../../root/project/ext/aws/module.lb:37' in function 'build':
        env.copy("freertos/FreeRTOS-Plus-TCP", "freertos_plus_tcp",

You can run the test locally: python3 tools/scripts/docs_modm_io_generator.py -t -c -j4

@mikewolfram
Copy link
Contributor Author

I know. But since that is the result of another pull request, how do I get it into this one? Update the submodule and commit?

@rleh
Copy link
Member

rleh commented Aug 25, 2020

Yes, you have to update and commit the submodule ext/aws/freertos.

@mikewolfram
Copy link
Contributor Author

Yes, you have to update and commit the submodule ext/aws/freertos.

It shows my private repository master branch and I cannot get it changed. Help!!!

rleh
rleh previously requested changes Aug 26, 2020
ext/aws/FreeRTOSIPConfig.h.in Outdated Show resolved Hide resolved
@salkinium salkinium force-pushed the freertos_plus_tcp_module branch from aa37354 to 7e7ac6f Compare August 30, 2020 12:55
@salkinium
Copy link
Member

I've squashed your commits.

This would allow the user to add additional defines for FreeRTOS TCP. Or maybe better use a collector in the project.xml file? How would that look like and how to get them into FreeRTOSIPConfig.h?

This is great idea, so I added this to both configs. Please check if this works for you, then I'd like to merge this PR.

(lbuild collectors are only useful when additional side-effects from modm are required, for example, the build system flags are reformatted into the specific build systems. For this, it's completely overkill, your solution is way better.)

ext/aws/module.lb Outdated Show resolved Hide resolved
@mikewolfram
Copy link
Contributor Author

Is it ok to assume that the compile has the __has_include() function? What if not?

@salkinium
Copy link
Member

Is it ok to assume that the compile has the __has_include() function? What if not?

Yes, it's part of C++17 and both GCC and LLVM have it implemented.

@salkinium salkinium force-pushed the freertos_plus_tcp_module branch from 7e7ac6f to 821677b Compare August 30, 2020 13:27
@salkinium
Copy link
Member

I've removed the options and added some documentation, does this work for you?

@mikewolfram
Copy link
Contributor Author

Now a test is failing, what is wrong?

@salkinium
Copy link
Member

Now a test is failing, what is wrong?

The arm-eabi-gcc download link isn't working in the CI. It's working for me locally, so I'm just going to ignore it for now.

@salkinium salkinium merged commit 821677b into modm-io:develop Aug 30, 2020
@salkinium
Copy link
Member

(The macOS CI is not part of the required tests, since it has been proven a bit unreliable.)

@mikewolfram mikewolfram deleted the freertos_plus_tcp_module branch August 31, 2020 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants