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

Would you accept a PR that added a compilelib command? #936

Open
paniag opened this issue Sep 3, 2020 · 13 comments
Open

Would you accept a PR that added a compilelib command? #936

paniag opened this issue Sep 3, 2020 · 13 comments
Assignees
Labels
criticality: low Of low impact topic: CLI Related to the command line interface type: enhancement Proposed improvement

Comments

@paniag
Copy link

paniag commented Sep 3, 2020

The name of the command isn't important, but the functionality is. Our use case is for testing large and complicated sketches as native applications on the host.

Desired functionality: build a library as a normal Arduino build that omits only main.cpp.o from core.a. Toolchain configuration to build for the host would be provided through a user's custom platform.txt and board.txt.

Use case: keyboard.io develops keyboard firmware, and we would love to have a native environment where we can do sophisticated testing involving simulation of the hardware. The firmware is highly customizable and extensible, but testing it is currently a highly manual endeavor involving flashing firmware onto a physical keyboard and actually typing. In particular this would allow us to incorporate arduino-cli into a larger build system and link against test and simulation libraries not built by any form of the Arduino build process. Concretely, we'd like to link against googletest and we have already put together a minimalist "mock" arduino core for native compilation.

Forking: We'd like to avoid forking arduino-cli due to the maintenance cost of keeping it up to date. We'd be happy to do the development - it should be a relatively simple change to make, could even be a flag passed to the existing compile command. We think this would be helpful to the community as well since it provides greater flexibility, so we're hoping you would be willing to take on maintenance of this feature.

We'd love to hear your thoughts on this!

@ubidefeo
Copy link

ubidefeo commented Sep 3, 2020

@paniag
thank you for the the proposal/suggestion.
We've been following your project :)
I'll have to check with @cmaglie . You seem to have a specific and detailed case and it sounds like something others might need.
I like how some of our users push the boundaries :D

@matthijskooijman
Copy link
Collaborator

Interesting project. I happen to be looking at a similar thing for a customer (running firmware on a regular PC using mocks for the Arduino API / hardware), so I'm much interested in this as well. Here's some misc thoughts:

  1. Did you see the arduino_ci project? This is a similar thing, intended to run unittests against libraries on a regular PC. They also have a mock Arduino library, but use some custom Ruby scripts as the compilation driver, rather than arduino-cli.
  2. A few similar projects that I've collected over the years (for inspiration, I think all of them are unmaintained and outdated) are: https://github.com/susundberg/arduino-simple-unittest, https://github.com/maniacbug/ncore, http://www.avr-developers.com/mm/arduino_unix.html, https://searduino.wordpress.com/implemented-features/, https://github.com/georgeredinger/ardutest, https://github.com/ikeyasu/arduino-gmock-sample, https://github.com/matthijskooijman/ArduinoUnix
  3. I happened to have suggested your exact same approach (Define a "PC"-Arduino-core with mocks and let arduino-cli drive compilation with it).
  4. You say you need arduino-cli to build omitting main.cpp.o. But if you want to always omit this file, and have a custom Arduino core, why not just omit main.cpp from the core? I'm not sure I understand your need, though, since your build still needs a main function, right? Or maybe you want arduino-cli to build just the library, and later include it in a bigger build system? Then I wonder, why not just let the bigger system just build the arduino library and mock core as well?
  5. You could also put a #if !defined(OMIT_MAIN) around your entire main.cpp file, and then run arduino-cli --build-properties build.extra_flags=-DOMIT_MAIN or so (but also see Provide dedicated sets of "extra flags" properties for platform developers and users #846 for some caveats about the property name, though with a custom core, you can use whatever property you want).
  6. You say you need to compile a library (rather than a sketch), but isn't that a matter of creating an empty sketch that only includes your library header file? By empty, I mean really empty, not even setup/loop, provided your customized core does not need those.
  7. Are you planning to publish your work (in particular the Arduino core/platform would be interesting for me)? Or maybe you already have a WIP-version somewhere?

@paniag
Copy link
Author

paniag commented Sep 3, 2020

Thanks for your comments!

tl;dr: Your comments led me to try another configuration method, and it worked! While I was able to achieve the desired result (building a library from a sketch to link into a larger binary), personally I did not find this method to be easily
discovered. So I would like to keep some form of this proposal open. Read on for details.

Highlights

  • Stripping out main.cpp.o is not strictly required for our usecase, but seems to make for a more general solution.
  • There is tremendous value in using the official Arduino toolchain whenever working with code intended to eventually run on Arduino.
  • We have a custom native core, but I doubt it is of publication quality yet. If there's more general interest, we can scope out the requirements for such a project and move on from there.

Detailed response
@matthijskooijman Very cool to find that we're not alone in wanting this kind of setup! We should chat; we might well be able to learn from one another. See https://github.com/keyboardio/Kaleidoscope/wiki#community for how to get in touch with us. Please find point by point responses below.

  1. I haven't looked at arduino_ci personally. I think it may have been considered in another context, but I'm not sure. However, I would strongly prefer a solution using an official (rather than custom) Arduino build system. The reasons are quite simple: custom systems may drift from the spec over time (or even just from the very beginning), projects may die or suffer bit rot from inadequate attention, etc. A solution using standard Arduino tooling avoids most of these problems, even if using a custom core.
  2. Thanks for the references. I've seen a couple of these before, but many are new. All of the Arduino (unit) testing libraries that I've come across are relatively low-level and seem better suited to simpler sketches. When scaled up to a higher level of abstraction, I believe those tests would quickly become unreadable and brittle. Such has already happened in our initial efforts to develop a testing framework. A (perhaps secondary) goal of our testing system is testing as documentation - for plugin developers or more advanced end users customizing their firmware. In past projects (but not yet in the embedded space) I've found googletest to provide excellent abstractions: high level, readable, flexible, and extensible. I'd very much like to incorporate those properties as we develop our automated testing solution.
  3. That's awesome. I'll give arduino_ci a closer look and see if I can contribute anything there. FWIW, keyboard.io's solution involves exactly the configuration process you describe in #147 in arduino_ci. You can see our platform.txt and boards.txt files here. The native core is even there under virtual/cores/arduino, but please note that it is very much a WIP and probably hasn't been polished to standards suitable with sharing it as any sort of recommended solution. Nonetheless, maybe you can take inspiration from it.
  4. You raise many very good points. Indeed, it is not strictly necessary - in our current setup - even to omit main.cpp.o from core.a. However, doing so provides greater flexibility as you can link against another object file or archive that provides main() without forking your core. This allows for using the same core with different testing frameworks. In our case, we have a "playback" style simulator which could be factored separately from a collection of unit and integration tests. Indeed, one way or another the final executable built needs a main() function definition. The use case for the functionality in this proposal is in fact to integrate arduino-cli into a larger build system. We're currently using the legacy arduino-builder (and currently looking at migrating to arduino-cli), but we've found it has trouble compiling some non-Arduino libraries (i.e. googletest). Tbh, I'm not sure what the exact problem is, but it strikes me as a more general solution to have arduino-cli handle the Arduino library part of the build and have a native build system handle the rest. Regarding why we wouldn't want the native ("bigger") build system to build the Arduino core and libraries, please see my response to 1.
  5. You are quite right. I think that didn't occur to me simply due to my aversion to conditional compilation. We already have tons of that in the codebase, and (at least) I consider it tech debt. Just not including main.cpp in the core or having the option to strip it out in an arduino-cli build seem like cleaner solutions to me.
  6. Couple of points here. First, compiling an empty sketch is not, in general, the same as compiling a library as the default Arduino build process produces an executable. Second, the sketch is a required component of our build process. It fully describes a particular build of firmware by specifying and providing configuration for plugins. Check out our documentation for more details. In our particular case, we do not need a custom loop function to be specified in the sketch because the firmware is designed to work by having the Arduino loop() function simply call a function that runs a single cycle of our firmware (key switch scan, event handling, plugin execution, etc).
  7. I'm not sure what the ideas are around publishing, but you are welcome to look at our codebase at Kaleidoscope and Kaleidoscope-Bundle-Keyboardio. Please consider anything in there as a WIP rather than a recommended / published solution ready for consumption outside of our project. As before though, you may be able to learn or gain inspiration from it.

Summary

  • It sounds like there is utility beyond keyboard.io for this functionality. While it is achievable through highly customized configuration, that probably puts it beyond the reach (or at least the sight) of many less experienced members of the Arduino community.
  • While I proposed a command in the title, I think a new flag on the existing compile command is a smaller and more easily maintained change.
  • Alternatively, documentation of how to customize for this use case could be added, but it seems really unnecessary to program humans when we could just as well (and perhaps even more easily and effectively) program machines.
  • We do have a custom core that should not be considered publication-ready, as well as customized platform.txt and boards.txt configs. Both could provide some inspiration.

Conclusion: I believe this proposal is still useful and relevant. Please let me know your thoughts.

@matthijskooijman
Copy link
Collaborator

Thanks for your additional clarifications. I believe I agree with pretty much everything you said :-)

I initially thought that your suggestion was to add a command that only omits the main.cpp.o file, but now it is clear that another (possibly more important) part of your suggestion is that no executable will be produced, but an object file (or whatever the format is exactly) that can be later included in another build.

Thanks for sharing your current virtual core, that might indeed be useful inspiration. One more thought I want to share is that if such a core is further developed, it should probably have a flexible and powerful way to mock Arduino API behaviour. arduino_ci has a "Godmode" class for this, that allows e.g. setting pin values, but I've recently wrote down some thoughts about making this more configurable, that you might find interesting (though it could very well be that the googletest framework solves some or all of this, maybe even in a more powerful way?

I have some more thoughts below. It has become a bit of a braindump, apologies for the lack of structure, but hopefully it will be useful nonetheless.

I do wonder how much of this and how it should be included in arduino-cli directly. I.e. it would seem wrong if arduino-cli would hardcode the "main.cpp.o" filename and omit that from the file list, so that should probably be somehow configured in platform.txt. And it would be wrong if arduino-cli would somehow change the linker commandline to not generate an executable, for that platform.txt would need to offer a different linker command.

So platform.txt would need to modified in any case, so do we really need this option (I'm not arguing we don't, I'm just asking)? I can imagine it can make things easier, but it would have to be used in tandem with modifications in platform.txt. In other words, this option will not allow compiling into non-executable object for arbitrary for arbitrary cores. I initially thought that this would only be useful for such "virtual" cores, but thinking on this more, I can also see a usecase where you would build a non-executable object for an actual microcontroller, which you then link with a custom main function, or maybe include in a bigger application, maybe even using an embedded OS of some sort.

So then the question is: Is a single "output executable or linkable library" flag sufficient, or does that assume too much? I can, for example, imagine that a core could define multiple modes. The default would of course be an executable, but I can imagine that a core could operate in a mode where it omits main(), but still exposes an arduino_init() function that initializes the hardware needed (e.g. timers), or another mode where it does no initialization at all? Maybe this is could of course just be some extra defines or build properties, but maybe all of this could be build properties?

Maybe the question is: If this option is added, how should it influence the build process exactly (in terms of omitting which filenames from compilation, replacing which recipes from platform.txt which which other ones, etc.)? How can this be done in a way that's easy to use, but also remains flexible for the platform developer to implement what they need?

@paniag
Copy link
Author

paniag commented Sep 18, 2020

@matthijskooijman I think your comments are entirely fair.

If the core symbols (like main) in main.cpp are weak, then actually excluding main.cpp.o would be unnecessary. I don't believe this is the case for the standard arduino core though.

I'm not sure I agree that specifically excluding main.cpp.o is an unnecessarily special case. Any program's main.cpp is a special file already, responsible for providing the entry point to an executable. On those grounds alone, I think it makes sense to single out main.cpp.o. That said, I agree it could be a convenient feature to drop object files from core.a in a configurable way if a use case arises for having a partially-virtual core. Maybe something like that could be useful in porting Arduino to new embedded platforms. I'm not sure. I'm also not sure that the standard Arduino core is designed with this level of modularity in mind in the first place, so it may be a moot point. It would be good to hear from someone from Arduino (or at least someone more knowledgeable than I am) about this.

Ultimately, I agree with your point that if someone is doing sufficiently tricky things to need to modify or swap out the standard core then they are already modifying platform.txt and the value in having a specific option to control that behavior outside of platform.txt has value primarily in terms of convenience.

That said, if the above mentioned case of selectively dropping out pieces of core.a is useful, I don't believe that functionality is currently configurable even via platform.txt. My understanding is that arduino-builder/arduino-cli has full control over how the core is built. Configuring platform.txt just enables you to drop or swap out the whole thing. Feel free to correct me if I'm mistaken.

To summarize:

  • platform.txt does already provide sufficient configuration to achieve my original goal stated in this issue.
  • Perhaps the more advanced use cases like porting Arduino to a new/custom embedded platform or embedding it in a larger OS justify either changing the request in this issue or creating a new one to cover that use case.
  • I think specific configuration methods should only be discussed after the use case is nailed down, now that it is potentially changing somewhat from the original request.

What do you think?

@matthijskooijman
Copy link
Collaborator

If the core symbols (like main) in main.cpp are weak, then actually excluding main.cpp.o would be unnecessary. I don't believe this is the case for the standard arduino core though.

This is not the case for the Arduino core, but that could be changed (

I'm not sure I agree that specifically excluding main.cpp.o is an unnecessarily special case. Any program's main.cpp is a special file already, responsible for providing the entry point to an executable. On those grounds alone, I think it makes sense to single out main.cpp.o.

You have a point, but I'm just saying that from the arduino-cli perspective, it is not special. It's just the file that happens to contain the main() function, but a core could also put the main() function in a NotMain.cpp, or even leave it out entirely if the particular architecture does not need it (i.e. has another entrypoint). The latter is a bit far-fetched, but it shows that there aren't much assumptions arduino-cli can really do here.

My understanding is that arduino-builder/arduino-cli has full control over how the core is built.

This is partially true: arduino-cli handles which files are included in compilation, which are then compiled using the normal c/cpp compiler recipes in platform.txt and combined into core.a using another recipe from platform.txt IIRC. But indeed, there is no way for a platform.txt to omit specific files from the core (other than using hacks like wrapping the compiler with a script that drops things from the commandline, but I'm going to ignore that).

What do you think?

Agreed, getting the usecases clear is probably needed first.

Since my last post, I've been working more on my project and I'm also feeling the need for a compilelib option a bit more. I'm now relying on a custom Makefile that sets up all include paths to libraries similarly to what Arduino does, which works, but it would be nicer if I could just pass my sketch to arduino-cli and have it output some .o file that I could then link against my virtual code (i.e. simulator main function, or unit tests) to produce the final executable.

However, writing this, I realize that my simulator/unittest code actually references types and code from the sketch and libraries, so my unittest code should probably be compiled by arduino-cli as well (or at least with the same or similar compiler flags and include paths). That could significantly change the requirements: Instead of producing a .o file to be linked externally, I would need to externally insert code into the build process (which could be a matter of a conditional include in the sketch, with the unittest code in an Arduino library, but I'm not sure that seems right).

That's a significantly different usecase from the "embed into other OS", maybe, since that would typically just need a few functions defined by the Arduino sketch to be called by the OS. Though maybe tighter integration could also make sense here, but then you might have code that relies on the (include paths for) the Arduino sketch and the external OS, so then who should compile this code without duplicating flags and include paths? Should Arduino-cli maybe just allow exporting include flags? Hm.

@ianfixes
Copy link

ianfixes commented Nov 20, 2020

arduino_ci owner here, subscribing for interest. (I moved the project and your GitHub links to it in this thread are broken, s/ianfixes/Arduino-CI/)

I haven't looked at arduino_ci personally. I think it may have been considered in another context, but I'm not sure. However, I would strongly prefer a solution using an official (rather than custom) Arduino build system. The reasons are quite simple: custom systems may drift from the spec over time (or even just from the very beginning)

arduino_ci began as a prototype to see whether hardware-less CI testing was even possible. At the time, arduino-cli was very limited in its functionality so I had to wrangle the IDE in some very roundabout ways to get it working. The drift you were concerned about is/was very real and very frustrating. (The lack of official unit testing capability was really my only motivation for the project in the first place.)

Now that arduino-cli covers most of my use cases (in wonderful JSON format) I'm in the process of adopting it as ardunio_ci's backend: Arduino-CI/arduino_ci#218

When that's complete, I'll be very curious to see how much of my codebase is useful in support of this issue.

@ianfixes
Copy link

I've completed the backend swap of arduino_ci from the graphical IDE to arduino-cli version 0.13.0. I love it! Plus, it can now support a GitHub Action for testing Arduino.

From here, I'd like to look more closely at coordinating with this project, to hopefully get backend support for some of the values I'm simply guessing at right now (e.g. the proper compiler flags for various platforms). I'm sure this issue thread isn't the right forum for this; can you point me to the right one?

@matthijskooijman
Copy link
Collaborator

I'm sure this issue thread isn't the right forum for this; can you point me to the right one?

If there is no existing issue that fits the bill, I suspect the best approach is to just create a new issue to describe what you need, and we can always point to another issue, split into multiple issues, etc. from there.

@ubidefeo
Copy link

@ianfixes we're very happy you like what we're doing here, it really means a lot for the team.
As @matthijskooijman said, feel free to post an issue and one of us will definitely go through it.
We tend keep an eye notifications on a few repos, this being one of them ;)

@ianfixes
Copy link

ianfixes commented Mar 30, 2021

The github actions bot closed a lot of issues that I had submitted, with no explanation. Was this by design? I'm still interested in getting official "will consider this" / "wontfix" on any of those unceremoniously-closed issues, and I apologize for asking here but I don't know where else to direct the question.

@per1234
Copy link
Contributor

per1234 commented Mar 30, 2021

Hi @ianfixes. I apologize for the confusion. I seem to have broken the "stale-bot" workflow somehow. This was never intended to be closed.

@per1234 per1234 reopened this Mar 30, 2021
@ianfixes
Copy link

Thanks for clarifying -- it looks like you restored a lot of issues. I didn't realize how many things I was subscribed to in this repo until that happened :)

@rsora rsora removed the topic: CLI label Sep 16, 2021
@rsora rsora added topic: core topic: CLI Related to the command line interface labels Sep 22, 2021
@rsora rsora assigned rsora and unassigned rsora Oct 21, 2021
@ubidefeo ubidefeo added the criticality: low Of low impact label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
criticality: low Of low impact topic: CLI Related to the command line interface type: enhancement Proposed improvement
Projects
None yet
Development

No branches or pull requests

9 participants