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

[scons] Simplify Scons tooling, misc. fixes and improvements #558

Merged
merged 10 commits into from
Feb 26, 2021

Conversation

salkinium
Copy link
Member

@salkinium salkinium commented Feb 14, 2021

I got annoyed with our bloated SCons tooling and our overly verbose logging. So I changed a few things:

  • Add a floating point ABI option to :platform:cortex-m: Can be useful for regulating IRQ latency on M4/M7.
  • Allow the artifact store to be remapped: This allows ELF files to be committed into a repository and accessed later for post-mortem debugging.
  • Allow choosing a different firmware={GNU Build ID or path/to/filename.elf} for relevant SCons commands (program, debug, listings etc).
  • Validate :build:libaries format: -lname.a for example won't be properly recognized by the linker.
  • Centralization of all SCons COMSTR: Makes it easier to maintain and keep beautiful.
  • Fixing SCons buildpath relocation, it was off-by-one folder: build/modm/modm/src -> build/modm/src.
  • Using shorter paths for logging when it makes sense. I relocated the compilation paths to the buildpath
  • Copying dependency from DLR SCons Tools: We were only using parts of utils, buildpath and gcc, everything else has already been replaced.
  • Using relative paths in ELF file so that ELF files can be exchanged between devices/repos
  • Add scons hex target back and clean .bin, .hex and .lss targets on scons -c
  • Update documentation

cc @rleh @chris-durand

@salkinium
Copy link
Member Author

Relative debug file paths are not a build system issue, but a compiler "feature" controlled by the -ffile-prefix-map=old=new flag. Using this I can remove the absolute paths, and make the paths relative to the project.xml. Debugging still works fine since GDB is invoked from the project.xml path as well. However: The stdlibc++ headers/sources are of course linked in by the compiler itself, so we're going to have to remove them as well, but then also tell GDB were to find them?

An example listing:

./modm/src/modm/processing/timer/periodic_timer.hpp:62
 80004ca:	428d      	cmp	r5, r1
 80004cc:	d004      	beq.n	80004d8 <main+0x164>
 80004ce:	3301      	adds	r3, #1
_ZNSt6chronossImSt5ratioILx1ELx1000EEmS2_EEDaRKNS_8durationIT_T0_EERKNS3_IT1_T2_EE():
/usr/local/Cellar/arm-gcc-bin/10-2020-q4-major/arm-none-eabi/include/c++/10.2.1/chrono:682
 80004d0:	3a01      	subs	r2, #1
 80004d2:	428d      	cmp	r5, r1
 80004d4:	d300      	bcc.n	80004d8 <main+0x164>
 80004d6:	3202      	adds	r2, #2
_ZN4modm20GenericPeriodicTimerINS_6chrono11milli_clockENSt6chrono8durationImSt5ratioILx1ELx1000EEEEE7executeEv():
./modm/src/modm/processing/timer/periodic_timer.hpp:63
					if (diff < this->_interval) break;
 80004d8:	0025      	movs	r5, r4
 80004da:	35f5      	adds	r5, #245	; 0xf5
 80004dc:	35ff      	adds	r5, #255	; 0xff
 80004de:	3201      	adds	r2, #1

@salkinium
Copy link
Member Author

salkinium commented Feb 14, 2021

Found a solution to make all compiler paths relative too, should even be portable. Still have to fix CMake and tell GDB where to look for compiler sources.

@salkinium
Copy link
Member Author

All without any hardcoded paths, the compiler source directory is detected at runtime:

@salkinium salkinium force-pushed the feature/remove-dlr-scons-tools branch from acc5338 to 4aaa478 Compare February 15, 2021 19:02
@@ -59,6 +59,7 @@ program-bmp: build
ui?=tui
debug: build
@python3 modm/modm_tools/gdb.py -x modm/gdbinit -x modm/openocd_gdbinit \
-ex "dir $(dir $(realpath $(dir $(realpath $(shell which arm-none-eabi-gcc)))))" \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: This Makefile will get a make over via the ::make generator, so this is currently more of an ugly placeholder.

@salkinium
Copy link
Member Author

salkinium commented Feb 15, 2021

Which NUCLEO boards do you have @rleh @chris-durand? I want to exchange ELF files to test the relative paths for debugging. I have these:

  • NUCLEO-F411RE
  • NUCLEO-F401RE
  • NUCLEO-F334R8
  • NUCLEO-F103RB
  • NUCLEO-F091RC
  • NUCLEO-L476RG
  • NUCLEO-L152RE
  • NUCLEO-F446RE
  • NUCLEO-F072RB
  • NUCLEO-F429ZI

Update: Actually, never mind, I can simply test this myself by moving the modm folder around…

@salkinium salkinium force-pushed the feature/remove-dlr-scons-tools branch from 4aaa478 to 2d5c6c2 Compare February 15, 2021 22:47
suffix = '.cpp',
src_suffix = '.pbm',
emitter = bitmap_emitter,
single_source = True)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for these changes is that env.Action() did not resolve the COMSTR properly (just blank $SOURCE and $TARGET), hence the use of a function before, while using SCons.Action.Action() (or just Action() via import) did resolve this properly. I couldn't find this as documented behavior, perhaps it's a bug, perhaps not… who knows with SCons.

@salkinium salkinium force-pushed the feature/remove-dlr-scons-tools branch 2 times, most recently from d198504 to d3f834c Compare February 16, 2021 23:59
@salkinium
Copy link
Member Author

I added the firmware={GNU Build ID or path/to/firmware.elf} argument to all applicable commands (especially all scons debug command) so that the artifact path makes more sense.

@salkinium
Copy link
Member Author

FYI: I'm going to merge this tonight unless changes are requested. @rleh @chris-durand

@rleh
Copy link
Member

rleh commented Feb 17, 2021

I won't be able to do a review before next week.

@salkinium
Copy link
Member Author

Ok, then I'll wait until you've reviewed. 🧐

@chris-durand
Copy link
Member

I also don't have time to review it right now. This change seems quite time-consuming to review if you haven't worked on that code before. I will have a look during the weekend.

@salkinium salkinium force-pushed the feature/remove-dlr-scons-tools branch from d3f834c to e59adae Compare February 18, 2021 16:57
@salkinium salkinium linked an issue Feb 18, 2021 that may be closed by this pull request
@salkinium
Copy link
Member Author

I recommend going commit by commit for the reviews, since they are more nicely structured than the whole blob.

@salkinium salkinium force-pushed the feature/remove-dlr-scons-tools branch from e59adae to 1fc3805 Compare February 26, 2021 02:19
Comment on lines +19 to +23
%% if core.startswith("avr")
env["COMPILERPREFIX"] = "avr-"
%% elif core.startswith("cortex-m")
env["COMPILERPREFIX"] = "arm-none-eabi-"
%% endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't scale well for e.g. RISC-V and/or non-GCC compilers, but is ok for now...

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@rleh rleh mentioned this pull request Feb 26, 2021
@salkinium salkinium merged commit 1fc3805 into modm-io:develop Feb 26, 2021
@salkinium salkinium deleted the feature/remove-dlr-scons-tools branch March 3, 2021 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

scons gdb: examples/README.md out of date
3 participants