cvra / can-bootloader

The bootloader used to flash our CAN-connected boards
BSD 2-Clause "Simplified" License
143 stars 51 forks source link

Separate platform dependent code from core #26

Closed antoinealb closed 9 years ago

antoinealb commented 9 years ago

We need to separate platform specific from reusable code. See #22. The rationale is that we will reuse the bootloader for various boards, not only motor controller.

My suggestion is to place all board specific code into a platform/ folder and code that can be reused between some but not all platforms in an arch/ or port/ folder.

From a continuous integration point of view this is better than having a lot of different repositories because we can setup Travis to rebuild all ports on changes to core. It is also easier to manage development if it is somewhat centralized.

What do you think ?

msplr commented 9 years ago

Alternative: How about git branches for different boards? Thought, I'm ok with your suggestion. Maybe not such a complicated folder structure, since it really is not so much code.

antoinealb commented 9 years ago

I don't like git branches to manage different build configurations because it is easy for them to diverge and need very strong discipline to be regularly merged / rebased.

msplr commented 9 years ago

I see your point. The advantage of branches is, that the repo is not filled with every existing port. Though, like I said: I'm ok with your suggestion, if we can keep it tidy.

pierluca commented 9 years ago

Just throwing in my 2 cents. Strongly with @antoinealb here. Besides the risk of diverging with branches, I believe that ports are an integral part of the code base, not a version of said code. Conceptually, I can't see them as branches.

msplr commented 9 years ago

Here a proposal for the folder structure:

platform/
├── common
│   ├── armv7-m
│   │   └── boot.s
│   └── stm32f3
│       ├── can_interface.c
│       └── flash_writer.c
└── cvra-motorboard-v1
    ├── include.mk
    └── main.c

To build a specific target, the Makefile in the project root includes the platform/target/include.mk to get the needed C sources an include directories.

Stapelzeiger commented 9 years ago

Rename common to MCU? And I' not sure but i think the packager can build multiple targets, so the cvra-motorboard should contain a package.yml instead of the include.mk.

antoinealb commented 9 years ago

Could you show an example of what include.mk contains ? Is it a jinja template for packager ?

msplr commented 9 years ago

@antoinealb: It just adds the sources and include paths to the Makefile's variables. Though the solution proposed by @Stapelzeiger is probably better. Then we can create a packager-target for every platform.

msplr commented 9 years ago

How should the build system work for different targets? A different Makefile for each platform or one Makefile with different make targets? I think it would be nice if we can build all target platforms and run the test with one command.

antoinealb commented 9 years ago

I would say one makefile per platform and one "root" makefile which just run make into the different folders.

antoinealb commented 9 years ago

@nuft Do you think your plaftform dependent code will be ready for merge soon ?

msplr commented 9 years ago

I'm working on it. I'll make a pull request somewhen in the next few days.