Mobsya / aseba-target-thymio2

Thymio 2 firmware
8 stars 8 forks source link

CMake build script #29

Closed cor3ntin closed 6 years ago

cor3ntin commented 6 years ago

This patch add a cmake build script to build the firmware of thymio We will have to make more tests to ensure the builded firmware actually works, especially considering that I used the newest xc16 version

davidjsherman commented 6 years ago

Can ctest run MPLABX SIM or would the tests require a tethered robot?

cor3ntin commented 6 years ago

I don't think there is any test whatsoever 😞 - however, it would still be interesting to run the build to ensure it at least compiles.

davidjsherman commented 6 years ago

I can write a Jenkins file, but since you have a cascade of submodules in submodules I think it would be best to isolate the build in a container. Unfortunately I can't open an issue to remind me to write a Dockerfile because aseba-community/cmake-microchip isn't configured for issues.

cor3ntin commented 6 years ago

There isn't that many submodules. cmake-microchip is just a collection of generic cmake files to deal with the pic16 arch.

git clone --recursive https://github.com/aseba-community/aseba-target-thymio2 would fetch almost everything. However, we also needs aseba ( we look for both ../aseba and ./aseba + variable ASEBA_DIR) Aseba is not a submodule of aseba-target-thymio2 nor the otherway around maybe that should be the case.

davidjsherman commented 6 years ago

In any case I'm not sure it's a good idea to pollute the build nodes with the MPLABX toolchain: it needs to be installed by hand because of the license, and that breaks node deployment automation. I would prefer building a stable build environment for the firmware and isolating it in a container image.

davidjsherman commented 6 years ago

With a bit of tooling it should be possible to execute the tests in aseba/tests/compiler using MPLABX's sim30. One could write a drop-in replacement for asebatest that writes a command script for sim30. The command script for --memcmp would be something like

LD pic24super
LC thymio.coff
LP data/basic-arithmetic.txt
RP
E
DM
quit

that is, load the linker output for the firmware, load the memory, reset the processor, run the program, dump the memory, and stop. The wrapper script would reformat the memory dump and compare it to the expected memory contents in data/basic-arithmetic.dump.

@mbonani you must be an expert at this! does it seem plausible?

davidjsherman commented 6 years ago

With a normal installation in /opt/microchip and PATH correctly set to find xc16-gcc, I get

CMake Error at cmake-microchip/Modules/Platform/MicrochipMCU-C-XC16.cmake:24 (message):
  No Microchip XC16 compiler was found.  Please provide the path to an XC16
  installation on the command line, for example:

  cmake -DMICROCHIP_XC16_PATH=/opt/microchip/xc16/v1.25 .
Call Stack (most recent call first):
  cmake-microchip/Modules/Platform/MicrochipMCU-C.cmake:18 (include)
  /usr/share/cmake-3.7/Modules/CMakeDetermineCCompiler.cmake:28 (include)
  CMakeLists.txt:4 (project)

CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!
See also "/home/jenkins/aseba-target-thymio2/build/CMakeFiles/CMakeOutput.log".
davidjsherman commented 6 years ago

Also, these rules generate compilation commands with the -mcpu option defined redundantly:

/opt/microchip/xc16/bin/xc16-gcc  -DASEBA_ASSERT -DASEBA_LIMITED_MESSAGE_SIZE
-I/opt/microchip/xc16/PIC24F/h -I/home/jenkins/aseba-target-thymio2/molole
-I/home/jenkins/aseba-target-thymio2/usb_pic24 -I/home/jenkins/aseba-target-thymio2
-I/home/jenkins/aseba/vm -I/home/jenkins/aseba -I/opt/microchip/xc16/support/PIC24F
-I/opt/microchip/xc16/support/peripheral_24F  -mcpu=24FJ128GB106 -Os
-mlarge-code -fomit-frame-pointer -fno-strict-aliasing -fno-peephole2
-mcpu=24FJ128GB106 -o CMakeFiles/aseba-target-thymio2.dir/abo.c.obj
-c /home/jenkins/aseba-target-thymio2/abo.c

Even though the same value is provided, xc16 interprets this as multiple processors and fails:

[  2%] Building C object CMakeFiles/aseba-target-thymio2.dir/analog.c.obj
Options have been disabled due to restricted license
Visit http://www.microchip.com/MPLABXCcompilers to purchase a new key.
Multiple processors specified.
CMakeFiles/aseba-target-thymio2.dir/build.make:86: recipe for target 'CMakeFiles/aseba-target-thymio2.dir/analog.c.obj' failed
make[2]: *** [CMakeFiles/aseba-target-thymio2.dir/analog.c.obj] Error 255
CMakeFiles/Makefile2:67: recipe for target 'CMakeFiles/aseba-target-thymio2.dir/all' failed
make[1]: *** [CMakeFiles/aseba-target-thymio2.dir/all] Error 2
Makefile:83: recipe for target 'all' failed
make: *** [all] Error 2

If I manually run the command without the redundant -mcpu, the compilation (of abo.c) succeeds.

davidjsherman commented 6 years ago

Aseba should not be a submodule, since it may be already installed on the system.

The variable for the Aseba source directory should be aseba_DIR (rather that the current ASEBA_DIR) for compatibility with a future asebaConfig.cmake file, inspired by enkiConfig.cmake and dashelConfig.cmake.

cor3ntin commented 6 years ago

I may be wrong but aren't .cmake files for build directories ? It does not matter that aseba may be installed, as we need the actual source files since we are targeting a non-host architecture.

cor3ntin commented 6 years ago

Which version of x16-gcc are you using ? I did not encounter the issue. I'll try to fix it anyway.

cor3ntin commented 6 years ago

I must confess I didn't actually test with make and the bug was not present with a Ninja generator, for some reason.

(Generally speaking, we should encourage the use of Ninja)

davidjsherman commented 6 years ago

Re: https://github.com/aseba-community/aseba-target-thymio2/pull/29#issuecomment-356207783 Thanks! If you try it in the container defined by #31 then you will have the same environment as the CI build farm.

cor3ntin commented 6 years ago

Wow, thanks for doing that !

davidjsherman commented 6 years ago

To clarify https://github.com/aseba-community/aseba-target-thymio2/pull/29#issuecomment-356139614, I meant the Aseba source will be already installed on the development system. A person compiling the firmware will typically already have the Aseba source, and the development use case is someone changing the VM or the natives and wanting to test the resulting firmware in a robot. So cloning Aseba as a submodule would be counter-productive.

Perhaps the find_path for Aseba should define aseba_SOURCE_DIR to distinguish it from an aseba_DIR defined for an installed Aseba library.

davidjsherman commented 6 years ago

A minor question: since molole and cmake-microchip aren't really optional, did you consider git-subrepo instead of git-module? It might make updates to the CMake tooling easier to handle.

cor3ntin commented 6 years ago

Well if you don't think ninja is convenient enough - even though it's completely optional - I don't see why using an unofficial git plugin that would not be optional helps. molole and cmake-microchip are both good use cases for submodules.

mbonani commented 6 years ago

Thank you