Teacup_Firmware icon indicating copy to clipboard operation
Teacup_Firmware copied to clipboard

folder-structure discussion

Open tonnico opened this issue 8 years ago • 19 comments

Hi all,

since we are supporting more than just AVR we will have for example analog.c, analog-avr.c and analog-arm.c. With introducing stm32f411 we split analog-arm.c to analog-arm_lpc1114.c and analog-arm_stm32f411.c.

While some ArduinoIDEs were broken, since 1.6.11 we have a new 'feature'. Everything in the homefolder and everything in src/ and above will compile in the IDE.

The idea is, to have everything for AVR in a folder /src/AVR. Other stuff which are used only for ARM could be in a folder /lpc1114 or /stm32f411. The Makefile could be similar to the Makefile-SIM.

Where analog-stuff is pretty clean, the heater-stuff needs more work. heater.c/.h is controlling the heater via PWM. So we could introduce a pwm.c/.h platform-specific in the corresponding folders.

In the end it would be great to have only universal files in the homefolder.

Maybe you have also great ideas for naming-conventions?

tonnico avatar Feb 16 '17 06:02 tonnico

In some of my more recent projects, I have SRC/HAL/Board/... and SRC/HAL/CPU/... so both board specific stuff (eg pinouts and gpio configuration) and CPU specific stuff (hardware implementations of primitives) can be trivially hooked as needed using compile-time options. Main platform-independent code goes in src/ or maybe src/lib - Serial could be a wrapper library that overloads various stuff from whichever HW_serial is hooked from the HAL for example - and all the HAL drivers share common include headers so they present a unified interface to application code

On 16 Feb 2017 14:38, "Nico Tonnhofer" [email protected] wrote:

Hi all,

since we are supporting more than just AVR we will have for example analog.c, analog-avr.c and analog-arm.c. With introducing stm32f411 we split analog-arm.c to analog-arm_lpc1114.c and analog-arm_stm32f411.c.

While some ArduinoIDEs were broken, since 1.6.11 we have a new 'feature'. Everything in the homefolder and everything in src/ and above will compile in the IDE.

The idea is, to have everything for AVR in a folder /src/AVR. Other stuff which are used only for ARM could be in a folder /lpc1114 or /stm32f411. The Makefile could be similar to the Makefile-SIM.

Where analog-stuff is pretty clean, the heater-stuff needs more work. heater.c/.h is controlling the heater via PWM. So we could introduce a pwm.c/.h platform-specific in the corresponding folders.

In the end it would be great to have only universal files in the homefolder.

Maybe you have also great ideas for naming-conventions?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Traumflug/Teacup_Firmware/issues/265, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKBGl5ANNj9h14CtZFXFb9_VXicnjp6ks5rc-7HgaJpZM4MCpAR .

triffid avatar Feb 16 '17 10:02 triffid

With introducing stm32f411 we split analog-arm.c to analog-arm_lpc1114.c and analog-arm_stm32f411.c.

I'd prefer to not split in two levels here, but keep one level: analog-avr.c, analog-lpc.c, analog-stm.c, all on the same level. Also not using the exact chip type, because various LPCxxxx should be very similar if not equal, just as all the STM32xxxx variants.

Does Arduino IDE allow to move everything into src/ ? To reduce the distance between firmware and HAL sources. Like

Teacup_Firmware.ino
src/
src/analog.h
src/analog.c
src/delay.h
src/delay.c
...
src/avr/analog.c
src/avr/delay.c
...
src/lpc/analog.c
src/lpc/delay.c
...
src/stm/analog.c
src/atm/delay.c
...

IIRC, we also still have the long standing issue that the top level folder has do be named according to the .ino file, requiring people downloading the tar.gz archive to rename the folder after unpacking. Other firmwares solve this by having a folder of that name inside the top level folder and moving the .ino file inside there, making the top level folder almost empty. If you start moving sources around I'd suggest to solve that, too. Not sure wether Arduino IDE still insists on the folder name, though.

Traumflug avatar Feb 16 '17 13:02 Traumflug

Does Arduino IDE allow to move everything into src/ ? To reduce the distance between firmware and HAL sources. Like

Yes, this will work with Arduino > 1.6.11.

Not sure wether Arduino IDE still insists on the folder name, though.

It does. Foldername and ino-name must be the same.

When you have something like:

Teacup_Firmware.ino
src/
src/analog.h
src/analog.c
src/delay.h
src/delay.c
...
src/avr/analog.c
src/avr/delay.c
...
src-arm/lpc/analog.c
src-arm/lpc/delay.c
...
src-arm/stm/analog.c
src-arm/stm/delay.c
...

You don't need #ifdef's , because the Arduino-IDE will fetch any file in the src/-folders.

tonnico avatar Feb 16 '17 14:02 tonnico

When you have something like:

src-arm/lpc/analog.c
src-arm/lpc/delay.c
...
src-arm/stm/analog.c
src-arm/stm/delay.c
...

You don't need #ifdef's , because the Arduino-IDE will fetch any file in the src/-folders.

This disallows compiling ARM variants with Arduino IDE, right? In case it's right it doesn't look like a good plan, because compiling for ARM boards, even non-Arduino ARM boards, with Arduino IDE is quite possible. Just needs an Arduino IDE enthusiast to create something like a Gen7-ARM board file for that IDE.

Traumflug avatar Feb 16 '17 14:02 Traumflug

Ok, forgot this. The idea of @Traumflug will fit perfectly. You need only a define in the config for ARM (STM/LPC) or AVR.

And indeed, with a simple Arduino-Config file you could also compile any ARM-board.

tonnico avatar Feb 16 '17 14:02 tonnico

How should a makefile looks like in that case?

tonnico avatar Feb 17 '17 18:02 tonnico

Not sure wether Arduino IDE still insists on the folder name, though.

It does. Foldername and ino-name must be the same.

Some IDEs use a .pde instead of .ino. In my directory I have a symlink to work around this.

phord avatar Feb 17 '17 19:02 phord

How should a makefile looks like in that case?

Not sure what you mean. Is it this?

SOURCES = $(wildcard *.c) $(wildcard src/avr/*.c) $(wildcard src/arm/*.c)

If we can avoid the attic somehow, like putting all the files in src/ top-level folder, then you might be able to do only this:

SOURCES = $(wildcard *.c) $(wildcard src/*.c) $(wildcard src/*/*.c)

There are lots of makefile tricks to select one file instead of another, such as 'avr/clock.c' instead of 'arm/clock.c'. But I presume you do not mean that since it breaks with the IDE expectations.

phord avatar Feb 17 '17 20:02 phord

Uff... Yeah something like this. But also I need to modify the builddir.

This is my first attempt: https://github.com/Traumflug/Teacup_Firmware/commit/25c852b9ded07079fa5cbf Especially this part in Makefile-common:

CPU_OBJ = $(patsubst $(CPU_SOURCE_FOLDER)/%.c,$(BUILDDIR)/%.o,$(wildcard $(CPU_SOURCE_FOLDER)/*.c))
CPU_OBJ := $(patsubst %.c,%.o,$(CPU_OBJ))

STD_OBJ = $(filter-out $(wildcard $(CPU_SOURCE_FOLDER)/*.c),$(SOURCES))
STD_OBJ := $(patsubst %.c,$(BUILDDIR)/%.o,$(STD_OBJ))

OBJ := $(CPU_OBJ) $(STD_OBJ)

$(BUILDDIR)/%.o: %.c | $(BUILDDIR)
	@echo "  CC        $@"
	@$(CC) -c $(CFLAGS) -MMD -o $@ $<
	@$(call CCPOST)

$(BUILDDIR)/%.o: $(CPU_SOURCE_FOLDER)/%.c | $(BUILDDIR)
	@echo "  CC        $@"
	@$(CC) -c $(CFLAGS) -MMD -o $@ $<
	@$(call CCPOST)

Maybe a better idea instead this duplicated code?

This first attempt will work for Makefile and with ArduinoIDE 1.6.11.

tonnico avatar Feb 17 '17 21:02 tonnico

I think you should keep the path structure under BUILDDIR so you do not have filename collisions there. Consider if you have both src/clock.c and src/AVR/clock.c. Now you find ${BUILDDIR}/clock.o. Which source file is it from?

Instead you should wind up with ${BUILDDIR}/src/clock.o and ${BUILDDIR}/src/AVR/clock.o, and then you should link them both in.

phord avatar Feb 17 '17 22:02 phord

Makefiles are tricky things to understand and debug. I made changes to your move_src branch to implement the simple alternative I was suggesting. I think it covers all the cases, but I only tested it on avr and sim builds. Please have a look: e54f3fe

phord avatar Feb 20 '17 18:02 phord

Thanks @phord. Looks like pretty nice. I will take a look into it next weekend.

tonnico avatar Feb 21 '17 16:02 tonnico

Works perfectly. I force-pushed some changes. With that we can also move the simulator-code.

What do you think about the CMSIS-parts? Moving to the MCU-folders? Moving to a CMSIS-folder? The cmsis-corexx-files are general files for any cortex MCU.

tonnico avatar Feb 24 '17 20:02 tonnico

@phord Unfortunately adding the header-files with the -I flag will not work automatically with the arduino IDE.

tonnico avatar Feb 25 '17 19:02 tonnico

@wurstnase That's disappointing. Check the compiler flags the IDE generates to see if there's any common folder they can live in, maybe.

Or maybe we can rethink the layout.

phord avatar Feb 25 '17 19:02 phord

Nothing which you can do without modifying a file in the arduino-folders. And I can find currently only a temporary working version, with adding manually the temp folder in platform.txt.

tonnico avatar Feb 25 '17 21:02 tonnico

My current idea is to copy an extra file named "platform.local.txt" to the arduino folder. On Windows it is "C:\Users\username\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.17".

In that file you need to add the header-folders. e.g.: compiler.cpp.extra_flags="-I{build.path}/sketch/src"

tonnico avatar Mar 01 '17 10:03 tonnico

It's starting to sounds too complicated simply to appease the fickle Arduino IDE. Maybe we should stick with the "-avr.c" scheme.

phord avatar Mar 01 '17 22:03 phord

Yes, maybe. 😞

tonnico avatar Mar 02 '17 06:03 tonnico