image-et-son / p600fw

GliGli based Prophet 600 firmware upgrade
10 stars 4 forks source link

Build with docker #206

Open jmechnich opened 1 year ago

jmechnich commented 1 year ago

This PR adds a Dockerfile for building an image with a avr-gcc toolchain and a cmake configuration file for building. Usage is documented in BUILD.md.

WARNING: the python3 fix from PR #202 is included as well to make things work (so maybe rebase before merging). Rebased the PR, so all good now.

image-et-son commented 1 year ago

Hi, I am not so happy with the BUILD.md file. It is not accurate and there is more relevant information for the case without using docker. It would be one option to rename it BUILD_Docker.md and write it in such a way that it is clear that it only refers to Docker. The other option would be to incorporate all information on different compilation environments, incl. the info that is already in the documentation subfolder.

jmechnich commented 1 year ago

Actually, this PR introduces two changes at once:

IMHO the current Makefile is a bit convoluted for building the project and at some points inaccurate (which, for example, resulted in the build error fixed with the hack in #205). Using cmake improves stability and portability of the build system. I have not actually tested this, but with cmake, it is possible to create project files for Xcode and Visual Studio etc.

This is, of course, only a suggestion...if you prefer keeping the Makefile in the long run, it obviously makes sense to document the build process as well. Personally, I prefer concise Markdown files for basic usage and build instructions.

I am happy to discuss and include any changes you propose. Also, I will have another look at the information that is already in the documentation subfolder and check for what is missing.

Edit: I am not sure what relevant information you were referring to, could you point it out to me? The part referring to Windows seems to be outdated (maybe one would/could consider using the Windows Subsystem for Linux nowadays), and the Linux-specific part should be covered by this PR. I could add another paragraph for the build using the plain Makefile.