Skip to content

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Jun 11, 2023

No description provided.

@jbaldwin
Copy link
Owner

jbaldwin commented Jun 11, 2023

What's the reasoning for this change? Adding a header include directory for any project with the root inc/ for the project should work for any build system

This feels like a bad pattern for how includes should work since you could name clash, having the coro/ prefix on everything is very intentional

@dok-net
Copy link
Contributor Author

dok-net commented Jun 11, 2023

@jbaldwin Not so easy with the Arduino build system, actually. Clash? As I said before, it's not fully required, but generally implemented, that the double-quote syntax first searches next to the includer, before searching the include paths. So unless the header file is missing - and then there's still the C++ namespace as a guard -nothing bad can/should be able to happen. I believe this is used all the time.

@jbaldwin
Copy link
Owner

I'm not familiar with building on arduino, and unless if we can add a build step in github actions CI it seems like it'll be hard to not break it moving forward even if we did get it working now. Whats the compiler you are using? Gcc/g++ which is the only compiler this library currently supports has a compiler option to add a -I path when building so you could just add like -I /usr/local/lib/coro/ or something depending on where you put the files. Usually that is pretty standard but I've never really done much embedded work so I don't actually know. The cmakelists.txt file embedded int he project also exports the libraries include directory as well, I'm guessing cmake isn't available?

@dok-net
Copy link
Contributor Author

dok-net commented Jun 11, 2023

I am confident the includes work as they stand on Zephyr, but I have not been able to, nor have I seen anything in the documentation for Arduino libraries, to set include directories. Arduino libraries have to follow a convention, that's all. And that seem to make it impossible to create a library package that just works unless the headers located each other by relative path.
I don't quite understand what the negative implications that you mention might be - anytime a header is referenced from outside the inc/coro tree, it's the same as before, all that changes is how they locate each other for indirect inclusion, and I am confident that this works as expected and designed. Yes, both Zephyr and Arduino use gcc.

If there is any hint that this may really cause trouble, I'll drop the request, but if it could be done, I would gladly have it.

@dok-net
Copy link
Contributor Author

dok-net commented Jun 11, 2023

@jbaldwin You have convinced me :-) I'm withdrawing this PR. Could you please look at the Arduino PR #152 I have posted instead? It has a different solution, which you probably aren't going to be too excited about either. But if you don't mind it, I would still have to try and fix the CMakeLists.txt etc. to adopt the moved inc/coro...

@dok-net dok-net closed this Jun 11, 2023
@dok-net dok-net deleted the relincs branch June 11, 2023 18:31
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.

2 participants