so now the defines get added to platform_config.h

[brendena]

Goal - To get the defines in the platform_config.h How - I added a ["define"] tab to the board files. So i moved all the ['makefile'] defines to this new tag ['define']. You can see this in the bangleJS2 and the NRF52840DK. So this tag is completely optional and you can still do it the old way in the ['makefile'] tab it just wont show up in the platform page.

The other problem i had was that these defines were expected to be global. So certain things never included platform_config.h. So this was causing problems. So i figured the most full proof solution was to just these in a #ifndef and when you run the program it produces the define that will hide these specific defines when you actually build it.

So this change should be backwards compatible and not break anything.

The only one thing i don't like about this is i have a define tag in side the defines tag. Since some parameters have values and other don't.

[gfwilliams]

Thanks! My concern with this approach is it's going to involve having to change all of the BOARD.py files in this repo (and all the forks including @fanoush's) if we want this to work for everyone. Also some of the defines are for Nordic's SDK where as you note platform_config isn't included, so they'll always need passing on the command-line anyway.

So what about this: we modify the Makefile to call scripts/build_platform_config.py $(BOARD) $(HEADERFILENAME) $(DEFINES) and then modify build_platform_config to pick up those defines from the command line and create a block like you have currently done:

#ifndef ESPR_AUTOCOMPLETE_NOT_REQUIRED
#define ....
#endif

Then we just add -DESPR_AUTOCOMPLETE_NOT_REQUIRED when calling the compiler - so now those lines are basically ignored by the compiler, but something like vscode will pick them up and show the correct code greyed out?

[brendena]

That would work. The only other thing I would say. Is I do think it makes the board files a little nicer looking and it is optional. But it's good either way.

[fanoush]

maybe it could be done without change by gcc preprocessor, run once with all the commandline arguments, generate output and then include that (=add that to platform config header)

[fanoush]

$ echo | gcc -DXX=hello -E -dM -  -o all.h
$ grep hello all.h
#define XX hello

and BTW if you want it only for VS code then maybe it would not be needed to really use it as part of real build, just let vscode somehow to use it when it is parsing the sources

[gfwilliams]

I do think it makes the board files a little nicer looking

My concern is sometimes the defines go together with other lines, for example:

'DEFINES += -DCENTRAL_LINK_COUNT=2 -DNRF_SDH_BLE_CENTRAL_LINK_COUNT=2', # allow two outgoing connections at once
'LDFLAGS += -Xlinker --defsym=LD_APP_RAM_BASE=0x3660', # set RAM base to match MTU=131 + CENTRAL_LINK_COUNT=2

So splitting those up makes it less obvious that if you change CENTRAL_LINK_COUNT, LD_APP_RAM_BASE needs changing.

Also, while it can look better, adding another way to do things in config files (while some use old and some use new) is generally just going to confuse contributors more than having one way that's not quite so nice.

Just as a general thing, I've got many people using different versions of Espruino with minor changes in, some public, some not. If stuff gets changed and refactored, it can make a big difference to them if they're trying to update their branches to the latest version.

I know it's nice to try and get things looking easier to understand, and it may not seem like much, but people end up complaining to me when stuff changes - for instance I fully expect I'll get at least one email complaint just from changing SPI_COUNT to ESPR_SPI_COUNT

if you want it only for VS code

I'm pretty sure there are some settings in VSCode to handle this sort of thing already - once it's aware of the Makefile and how it should be executed I think it's got a way of figuring out what the defines are, so in a way maybe it's just a matter of configuring it properly so the Makefile handler knows that actually BOARD=BANGLEJS2 make should be called and not just make.

But I guess if there's a simple hack that doesn't create too many other issues (like adding the ifdef'd block to platform_config) then it's good to get in

[fanoush]

here is example without default gcc stuff in it (did not find gcc option to skip that stuff)

$ echo | gcc -E -dM - -o gccbasedefs.h
$ echo | gcc -DXX=hello -DYY=1 -DBANGLEJS2 -E -dM - | grep -vxf gccbasedefs.h
#define BANGLEJS2 1
#define XX hello
#define YY 1

[brendena]

Since the new commit has none of the old code i made a new pull request here.

[brendena]

So i tried to the GCC way of doing it and it did generate all the defines. But it generated in my mind too many of them. I want to also be able to go to this file and see what flags are enabled and with the GCC way there's just too many to see what's happening. On the bangle watch there's like 480 defines this way.

[fanoush]

So i tried to the GCC way of doing it and it did generate all the defines. But it generated in my mind too many of them.

oh so you don't actually want vscode to have all the missing ones to show (and navigate) the code properly?

with the GCC way there's just too many to see what's happening

you could | sort them if you are searching for specific name there. or just ctrl+click it in vscode when you see it

On the bangle watch there's like 480 defines this way.

480 even without the default ones that are skipped by example above? that seems like a lot indeed

[gfwilliams]

I'm with @brendena on this - using GCC is a neat idea, but I think just seeing the defines that the Makefile wants to pass to GCC on the CLI is neater

[fanoush]

sure but even with gcc it should be exactly those. So you don't want those that are printed when running make with V=1? like e.g.

-DGIT_COMMIT=0959f0999 -DNO_ASSERT -DRELEASE -DBUILDNUMBER="91" -DNRF52840Dongle -DCONFIG_GPIO_AS_PINRESET -DBOARD_PCA10059 -DNRF_SDH_BLE_GATT_MAX_MTU_SIZE=131 -DBLUETOOTH_SCAN_INTERVAL=40 -DBLUEOOTH_SCAN_WINDOW=30 -DBLUETOOTH_NAME_PREFIX="Dongle" -DNRF_USB=1 -DUSB -DNO_DUMP_HARDWARE_INITIALISATION -DNRF_BL_DFU_INSECURE=1 -DESPR_DCDC_ENABLE=1 -DESPR_LSE_ENABLE -DESPR_NO_LINE_NUMBERS=1 -DUSE_DEBUGGER -DUSE_TAB_COMPLETE -DUSE_HEATSHRINK -DUSE_GRAPHICS -DBLUETOOTH -DESPR_JIT -DS140 -DNRF_SD_BLE_API_VERSION=6 -D__HEAP_SIZE=0 -DBLE_STACK_SUPPORT_REQD -DSWI_DISABLE0 -DSOFTDEVICE_PRESENT -DNRF52_SERIES -DNRF52840_XXAA -DNRF52840 -DNRF5X -DNRF5X_SDK_15 -DUSE_APP_CONFIG -DESPR_BLUETOOTH_ANCS=1 -DARM -DLINK_TIME_OPTIMISATION -DEMBEDDED

That should be exactly those produced by gcc. but any way will do of course if it would actually help.

I am actually using vscode too and yes it is a bit annoying when it shows 'unreachable' code shaded or goes to wrong header when clicking symbol. but as you mentioned previously I was checking if it could be solved by running my 'make BOARD=XX' line but so far did not find it. So only some subset will not fix that. So it depends why we want this there in the first place.

[gfwilliams]

Yes, I know we can do that, but just adding $(DEFINES) on the build_platform_config.py command-line seems quite tidy.

I think the VS Code highlighting would be nice (also being able to see what things like BLUETOOTH_NAME_PREFIX are set to within the editor).

But also I find myself sometimes having to build with V=1 and then search the command-line just to sanity check what some of the defines are set to, so from this would help there too.