xmos / lib_i2c

I2C peripheral library
Other
3 stars 22 forks source link

Example makefiles need tidying and mix architecture and product naming #71

Open xross opened 3 years ago

xross commented 3 years ago

This issue covers a few point points on the example makefiles - as commented on https://github.com/xmos/lib_i2c/pull/68#pullrequestreview-599785771

The last two break the CONFIG=xxx option to xmake/xcommon. i.e. I should be able to build only for xcore200 with:

xmake CONFIG=xcore200

I provide an example below for app_simple_single_port_master. I believe this style should be replicated over all examples (and indeed examples in other libs as applicable)

# The APP_NAME variable determines the name of the final .xe file. It should
# not include the .xe postfix. If left blank the name will default to
# the project name

APP_NAME =

# The flags passed to xcc when building the application
# You can also set the following to override flags for a particular language:
#
#    XCC_XC_FLAGS, XCC_C_FLAGS, XCC_ASM_FLAGS, XCC_CPP_FLAGS
#
# If the variable XCC_MAP_FLAGS is set it overrides the flags passed to
# xcc for the final link (mapping) stage.

BUILD_FLAGS = -O2 -g -DDEBUG_PRINT_ENABLE=1 -report

XCC_FLAGS_xcore200 = $(BUILD_FLAGS)
XCC_FLAGS_xcoreai = $(BUILD_FLAGS)

# The TARGET variable determines what target system the application is
# compiled for. It either refers to an XN file in the source directories
# or a valid argument for the --target option when compiling.

ifeq ($(CONFIG),xcoreai)
TARGET = XCORE-AI-EXPLORER
else
TARGET = XCORE-200-EXPLORER
endif

# The USED_MODULES variable lists other module used by the application.

USED_MODULES = lib_i2c(>=6.0.0) lib_logging(>=2.1.0)

#=============================================================================
# The following part of the Makefile includes the common build infrastructure
# for compiling XMOS applications. You should not need to edit below here.

XMOS_MAKE_PATH ?= ../..
include $(XMOS_MAKE_PATH)/xcommon/module_xcommon/build/Makefile.common
lucianomartin commented 3 years ago

I agree we should remove the XCOREAI variable and to use the CONFIG build to decide what target to use. The problem I see with moving the flags outside the if statements is that if we type only xmake, we will create 2 builds, one named xcoreai and one xcore200, but they will both use the default target XCORE-200-EXPLORER. To avoid that and still having the xcore200 build as default, I propose this alternative solution:

# The APP_NAME variable determines the name of the final .xe file. It should
# not include the .xe postfix. If left blank the name will default to
# the project name

APP_NAME =

# The flags passed to xcc when building the application
# You can also set the following to override flags for a particular language:
#
#    XCC_XC_FLAGS, XCC_C_FLAGS, XCC_ASM_FLAGS, XCC_CPP_FLAGS
#
# If the variable XCC_MAP_FLAGS is set it overrides the flags passed to
# xcc for the final link (mapping) stage.

BUILD_FLAGS = -O2 -g -DDEBUG_PRINT_ENABLE=1 -report

# The TARGET variable determines what target system the application is
# compiled for. It either refers to an XN file in the source directories
# or a valid argument for the --target option when compiling.

ifeq ($(CONFIG),xcoreai)
XCC_FLAGS_xcoreai = $(BUILD_FLAGS)
TARGET = XCORE-AI-EXPLORER
else
XCC_FLAGS_xcore200 = $(BUILD_FLAGS)
TARGET = XCORE-200-EXPLORER
else ifeq ($(CONFIG),)
XCC_FLAGS_xcore200 = $(BUILD_FLAGS)
TARGET = XCORE-200-EXPLORER
else
$(error Invalid config specified)
endif

# The USED_MODULES variable lists other module used by the application.

USED_MODULES = lib_i2c(>=6.0.0) lib_logging(>=2.1.0)

#=============================================================================
# The following part of the Makefile includes the common build infrastructure
# for compiling XMOS applications. You should not need to edit below here.

XMOS_MAKE_PATH ?= ../..
include $(XMOS_MAKE_PATH)/xcommon/module_xcommon/build/Makefile.common

Would that be ok?

xross commented 3 years ago

I think there is some confusion about how the CONFIG system works in xcommon. See here for a working example: https://github.com/xmos/lib_xud/blob/develop/examples/AN00132_image_class/Makefile

Also the #else should be removed. xcommon will catch the bad config and helpfully print out a list of valid configs.