From d1c64997ccf1f9501b28bfd025fe12ebf81c9892 Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 12:13:00 +0100 Subject: [PATCH 1/7] Fix build problem caused by digipot header It looks like digipot.h was originally DigiPot.h. When commit 0a0a0e23 was merged this broke the build. Tests: - Project now builds --- include/stm32_vcu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/stm32_vcu.h b/include/stm32_vcu.h index f2c8c553..91a149ab 100644 --- a/include/stm32_vcu.h +++ b/include/stm32_vcu.h @@ -78,7 +78,7 @@ #include "dcdc.h" #include "TeslaDCDC.h" #include "BMW_E31.h" -#include "DigiPot.h" +#include "digipot.h" #include "shifter.h" #include "F30_Lever.h" #include "E65_Lever.h" From 0fe5fb063621703c67cc57e3675a65d11e47631f Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 12:22:09 +0100 Subject: [PATCH 2/7] Fix linker warning from newer toolchains Import commit 53afdb04 from stm32-sine which suppresses a linker warning: /usr/lib/gcc/arm-none-eabi/14.1.0/../../../../arm-none-eabi/bin/ld: warning: stm32_vcu has a LOAD segment with RWX permissions From the original commit: Due to the way that gcc generates .init_array sections containing C++ global object constructors it can result in ELF object file sections that are marked Read Write and eXecute. With binutils 2.39 or newer this will result in a warning. For an embedded use case like this inverter it can safely be suppressed. Suppressing the warning is not possible on earlier versions of binutils (GNU ld). The makefile now tests whether the link option will succeed before trying to use it. Tests: - Build test with gcc 14.1.0 and binutils 2.43 --- Makefile | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Makefile b/Makefile index fd73fcfa..ec3837cf 100644 --- a/Makefile +++ b/Makefile @@ -62,6 +62,24 @@ Q := @ NULL := 2>/dev/null endif +# try-run +# Usage: option = $(call try-run, command,option-ok,otherwise) +# Exit code chooses option. +try-run = $(shell set -e; \ + if ($(1)) >/dev/null 2>&1; \ + then echo "$(2)"; \ + else echo "$(3)"; \ + fi) + +# Test a linker (ld) option and return the gcc link command equivalent +comma := , +link_command := -Wl$(comma) +ld-option = $(call try-run, $(PREFIX)-ld $(1) -v,$(link_command)$(1)) + +# Test whether we can suppress a safe warning about rwx segments +# only supported on binutils 2.39 or later +LDFLAGS += $(call ld-option,--no-warn-rwx-segments) + all: directories images Debug:images Release: images From 3c3bafe2f6acf3c3b003e9f8012cbd3e32436e14 Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 14:08:19 +0100 Subject: [PATCH 3/7] Add Continuous Integration build using GitHub Actions There has been a lot of accidental build breakage with recent commits. To reduce the likelihood of this in future ensure that pushes and PRs are tested by running a clean build in a known clean environment and report any errors. This uses the same approach as the stm32-sine and ccs32clara projects for continuous integration. The readme is updated to display the current CI build state. It has also been update to reflect the changed build process for unit tests. --- .github/workflows/CI-build.yml | 47 ++++++++++++++++++++++++++++++++++ README.md | 9 ++++--- 2 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/CI-build.yml diff --git a/.github/workflows/CI-build.yml b/.github/workflows/CI-build.yml new file mode 100644 index 00000000..c8782db7 --- /dev/null +++ b/.github/workflows/CI-build.yml @@ -0,0 +1,47 @@ +name: CI +on: + push: + pull_request: + +jobs: + build: + name: build-linux + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Install build package dependencies + run: | + sudo apt-get update + sudo apt-get install gcc-arm-none-eabi + + - name: Build dependencies + run: | + echo "Number of processors:" `nproc` + make get-deps -j `nproc` + + - name: Build Stm32-vcu firmware + run: | + make + + - uses: actions/upload-artifact@v4 + with: + name: Stm32-vcu firmware binary + path: stm32_vcu.bin + + - uses: actions/upload-artifact@v4 + with: + name: Stm32-vcu firmware hex + path: stm32_vcu.hex + + - name: Build unit tests on host + run: | + make Test + + - name: Run unit tests on host + run: | + test/test_vcu diff --git a/README.md b/README.md index a7ec2189..234a6778 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,7 @@ # stm32-vcu + +[![Build status](../../actions/workflows/CI-build.yml/badge.svg)](../../actions/workflows/CI-build.yml) + Project based on the OpenInverter System by Johannes Huebner to provide a universal VCU (Vehicle Control Unit) for electric vehicle conversion projects. Please visit the development thread on the Openinverter Forum for more information : https://openinverter.org/forum/viewtopic.php?f=3&t=1277 @@ -51,15 +54,13 @@ Now you can compile stm32-vcu by typing # Tests -`cd tests` - Build the tests -`make` +`make Tests` Run the tests -`./test_vcu` +`./test/test_vcu` And upload it to your board using a JTAG/SWD adapter, the updater.py script or the esp8266 web interface From 467955d78b7503df0352accae92806e45c32e8c7 Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 14:47:09 +0100 Subject: [PATCH 4/7] Fix unit test build Fixing the compilation of throttle.cpp requires making utils.h suitable for cross-compilation. This is achieved by: - Including only the necessary .h files required to define types referenced in utils.h and moving all of the other includes to utils.cpp. It is best practice to do this in C/C++ to minimise dependencies. This requires missing includes to be added to BMW_E31.cpp and BMW_E39.cpp. - Make the utils::change() an inline header function so we don't need to write stubs for any STM32 specific code. The throttle now depends on params.cpp from libopeninv. This requires dummy versions of Param::Change() and errorListString to allow the link to complete. Neither are actually used by the tests Tests: - Build main STM32 firmware - Build unit tests - Running the unit test fails with: Assertion failed: Throttle::CalcThrottle(296, 0, false) > 0 in test_throttle.cpp : 92 Assertion failed: throtVal == 100 in test_throttle.cpp : 98 Assertion failed: throtVal == 100 in test_throttle.cpp : 104 --- include/utils.h | 24 ++++++++---------------- src/BMW_E31.cpp | 1 + src/BMW_E39.cpp | 1 + src/utils.cpp | 15 +++++++++------ test/Makefile | 2 +- test/test_throttle.cpp | 8 ++++++++ 6 files changed, 28 insertions(+), 23 deletions(-) diff --git a/include/utils.h b/include/utils.h index 6a69e4e5..601cec8f 100644 --- a/include/utils.h +++ b/include/utils.h @@ -1,27 +1,19 @@ #ifndef UTILS_H #define UTILS_H -#include "BMW_E65.h" -#include "my_fp.h" -#include "my_math.h" -#include "errormessage.h" -#include "params.h" -#include "digio.h" -#include -#include "canhardware.h" -#include "anain.h" -#include "throttle.h" -#include "isa_shunt.h" -#include "bmw_sbox.h" -#include "vag_sbox.h" #include "vehicle.h" #include "shifter.h" -#include -#include "iomatrix.h" + +#include "canhardware.h" +#include "errormessage.h" namespace utils { - int32_t change(int32_t, int32_t, int32_t, int32_t, int32_t); + inline int32_t change(int32_t x, int32_t in_min, int32_t in_max, int32_t out_min, int32_t out_max) + { + return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min; + } + float GetUserThrottleCommand(CanHardware*); float ProcessThrottle(int); float ProcessUdc(int); diff --git a/src/BMW_E31.cpp b/src/BMW_E31.cpp index e63fe98f..d8e3ac63 100644 --- a/src/BMW_E31.cpp +++ b/src/BMW_E31.cpp @@ -18,6 +18,7 @@ */ #include "BMW_E31.h" #include "hwinit.h" +#include "my_math.h" #include #include /* diff --git a/src/BMW_E39.cpp b/src/BMW_E39.cpp index fb90faee..51779c82 100644 --- a/src/BMW_E39.cpp +++ b/src/BMW_E39.cpp @@ -20,6 +20,7 @@ #include "stm32_can.h" #include "utils.h" #include "digio.h" +#include "my_math.h" static uint8_t counter_329 = 0; static uint8_t ABSMsg = 0; diff --git a/src/utils.cpp b/src/utils.cpp index 24f26fc1..1f393c19 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -1,5 +1,14 @@ #include "utils.h" +#include "iomatrix.h" +#include "throttle.h" +#include "vag_sbox.h" +#include "bmw_sbox.h" +#include "isa_shunt.h" +#include "my_math.h" +#include +#include + namespace utils { @@ -8,12 +17,6 @@ namespace utils float SOCVal=0; int32_t NetWh=0; - -int32_t change(int32_t x, int32_t in_min, int32_t in_max, int32_t out_min, int32_t out_max) -{ - return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min; -} - void PostErrorIfRunning(ERROR_MESSAGE_NUM err) { if (Param::GetInt(Param::opmode) == MOD_RUN) diff --git a/test/Makefile b/test/Makefile index 7cad069c..929c0898 100644 --- a/test/Makefile +++ b/test/Makefile @@ -6,7 +6,7 @@ CFLAGS = -std=c99 -g -I../include -I../libopeninv/include CPPFLAGS = -g -I../include -I../libopeninv/include LDFLAGS = -g BINARY = test_vcu -OBJS = test_main.o my_string.o throttle.o test_throttle.o +OBJS = test_main.o my_string.o params.o throttle.o test_throttle.o VPATH = ../src ../libopeninv/src all: $(BINARY) diff --git a/test/test_throttle.cpp b/test/test_throttle.cpp index 86198acb..7fd8bbdc 100644 --- a/test/test_throttle.cpp +++ b/test/test_throttle.cpp @@ -26,6 +26,14 @@ using namespace std; +void Param::Change(Param::PARAM_NUM p) +{ + // Dummy function - we ignore parameter changes in these tests +} + +// Include a dummy error list to allow the tests to link. It is unused. +const char* errorListString = ""; + static void TestSetup() { //percentage of deadzone From 9d956890519ed32f36d0ea92168f5d627f7fa747 Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 15:23:11 +0100 Subject: [PATCH 5/7] Allow unit tests to be debugged It is not possible to stop gdb on macros like the ASSERT() used in unit tests unless the project is built with -ggdb. Add a debug target for VS Code. --- .vscode/launch.json | 17 +++++++++++++++++ test/Makefile | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 9cc4469b..a0c0c2f2 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -36,6 +36,23 @@ ], "externalConsole": false, "cwd": "${workspaceFolder}" + }, + { + "name": "test_vcu debug with gdb", + "type": "cppdbg", + "request": "launch", + "program": "${workspaceFolder}/test/test_vcu", + "cwd": "${workspaceFolder}", + "setupCommands": [ + { + "description": "Enable pretty-printing for gdb", + "text": "-enable-pretty-printing", + "ignoreFailures": true + }, + { + "text": "set output-radix 16" + } + ] } ] } \ No newline at end of file diff --git a/test/Makefile b/test/Makefile index 929c0898..26be6ad1 100644 --- a/test/Makefile +++ b/test/Makefile @@ -2,8 +2,8 @@ CC = gcc CPP = g++ LD = g++ CP = cp -CFLAGS = -std=c99 -g -I../include -I../libopeninv/include -CPPFLAGS = -g -I../include -I../libopeninv/include +CFLAGS = -std=c99 -ggdb -I../include -I../libopeninv/include +CPPFLAGS = -ggdb -I../include -I../libopeninv/include LDFLAGS = -g BINARY = test_vcu OBJS = test_main.o my_string.o params.o throttle.o test_throttle.o From a8e9e3732aa477f4d5fb68d9f9cb19469c183908 Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 15:25:17 +0100 Subject: [PATCH 6/7] Fix unit tests by adding missing throttle default parameters The throttle calculation process now depends on the vehicle direction and maximum permitted throttle position being set. configure these during each test setup. Tests: - Build and run unit tests successfully --- test/test_throttle.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_throttle.cpp b/test/test_throttle.cpp index 7fd8bbdc..f9dee75f 100644 --- a/test/test_throttle.cpp +++ b/test/test_throttle.cpp @@ -40,6 +40,8 @@ static void TestSetup() Throttle::throtdead = 5; Throttle::potmin[0] = 100; Throttle::potmax[0] = 4000; + Throttle::throtmax = 100; + Param::SetInt(Param::dir, 1); } // TEMPERATURE DERATING @@ -90,7 +92,7 @@ static void TestCalcThrottleIs0WhenInDeadZone() { static void TestCalcThrottleIsAbove0WhenJustOutOfDeadZone() { //deadzone is first 5% of travel between 100 and 4000 - ASSERT(Throttle::CalcThrottle(296, 0, false) > 0); + ASSERT(Throttle::CalcThrottle(496, 0, false) > 0); } static void TestCalcThrottleIs100WhenMax() { From 33f52c58218aa23e0e55cfcaf481781ee6e05ecb Mon Sep 17 00:00:00 2001 From: "David J. Fiddes" Date: Tue, 24 Sep 2024 19:08:32 +0100 Subject: [PATCH 7/7] Fix warning in BMW_E65::GetGear Teh parameter outGear is currently unused so mark it as such to stop the compiler moaning. --- src/BMW_E65.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BMW_E65.cpp b/src/BMW_E65.cpp index 1243d9e4..9f2aa526 100644 --- a/src/BMW_E65.cpp +++ b/src/BMW_E65.cpp @@ -371,7 +371,7 @@ void BMW_E65::Engine_Data() } -bool BMW_E65::GetGear(Vehicle::gear& outGear) +bool BMW_E65::GetGear(Vehicle::gear& outGear __attribute__((unused))) { return false; }