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

[freertos-plus-tcp] Add includes #475

Merged
merged 4 commits into from
Sep 18, 2020

Conversation

strongly-typed
Copy link
Collaborator

There is no FreeRTOS plus TCP project yet in there so the code is not covered by the CI.
When adding <module>modm:freertos:tcp</module> the includes are missing

In file included from modm/ext/freertos_plus_tcp/FreeRTOS_DHCP.c:35:
modm/ext/freertos_plus_tcp/include/FreeRTOS_IP.h:138:10: fatal error: pack_struct_start.h: No such file or directory
  138 | #include "pack_struct_start.h"

What would be more elegant way to add the includes? Shall it depend on the compiler (GCC vs clang?) But only GCC is imported from the original source.

By the way, let's bump FreeRTOS module in modm to the current version.

@salkinium
Copy link
Member

What would be more elegant way to add the includes?

You could copy the files directly to the local folder. That way they don't spam up the include path.

Shall it depend on the compiler (GCC vs clang?) But only GCC is imported from the original source.

We don't support any other compiler than GCC right now. We'll deal with this when we add LLVM support in the (far) future.

@strongly-typed
Copy link
Collaborator Author

You could copy the files directly to the local folder. That way they don't spam up the include path.

Like so? But I was not able to identify which command copies these files already to portable:

find . -name "pack_struct_start.h"
./modm/ext/freertos_plus_tcp/include/pack_struct_start.h
./modm/ext/freertos_plus_tcp/portable/Compiler/GCC/pack_struct_start.h

ext/aws/module.lb Outdated Show resolved Hide resolved
@strongly-typed
Copy link
Collaborator Author

How are we dealing with the warnings?

image

image

image

image

@salkinium
Copy link
Member

How are we dealing with the warnings?

Yeah, that's an issue in general. You can attach compiler options to specific files (see :cmsis:dsp module), however, the build system can only translate these to source files, not header files (obviously).

So if you include a FreeRTOS header file in main.cpp for example, you still get the same warnings.

@strongly-typed
Copy link
Collaborator Author

Which warnings shall be fixed? Are these critical?

  • taking address of packed member of '' may result in an unaligned pointer value [-Waddress-of-packed-member]
  • #pragma warning "ipconfigTCP_MEM_STATS_MAX_ALLOCATION undefined?"

@salkinium
Copy link
Member

Which warnings shall be fixed? Are these critical?

I don't know. FreeRTOS is fairly complicated and lacks clear documentation for most things.

@strongly-typed
Copy link
Collaborator Author

I don't know. FreeRTOS is fairly complicated and lacks clear documentation for most things.

modmFreeRTOS ⏩💘

😎

@strongly-typed strongly-typed changed the title [draft] [freertos-plus-tcp] Add includes [freertos-plus-tcp] Add includes Sep 17, 2020
@salkinium
Copy link
Member

Could you cherry-pick the commit from #474 over to here?

@salkinium
Copy link
Member

salkinium commented Sep 17, 2020

What hardware are you testing this on? I think it would be neat to test this on the Nucleo-144 boards which have an integrated Ethernet port for like ~20€.

I have these lying around:

  • Nucleo-F429ZI
  • Disco-F746G
  • Nucleo-H743ZI (but unsupported by modm)

@strongly-typed
Copy link
Collaborator Author

What hardware are you testing this on?

Not yet testing. Just get the compiler to work and read through Mike's code.

  • Nucleo-F429ZI

I also have this board on my desk, but did not hook it up (yet) except for adjusting the example. I just moved the FreeRTOS plus TCP example to Nucleo-F429ZI, which makes more sense.

@strongly-typed strongly-typed marked this pull request as ready for review September 17, 2020 19:53
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested the malloc lock with all allocators in hardware.

@salkinium
Copy link
Member

Ready for squash and merge?

@strongly-typed
Copy link
Collaborator Author

RTM

@salkinium salkinium merged commit cb82eec into modm-io:develop Sep 18, 2020
@strongly-typed strongly-typed deleted the feature/freertostcp branch September 18, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants