Skip to content

Commit

Permalink
Fix unsigned CAN receive mapping for 32-bit values and associated uni…
Browse files Browse the repository at this point in the history
…t testing (#22)

* Import unit tests from stm32-sine

Pull in stm32-sine unit tests from commit 1b43e4c.

Remove stm32-sine specific tests.

Add minimal param_prj.h and hwdefs.h and missing
libopencm3 stubs to get the unit tests to link and run.

Construct a GitHub action to build just the unit tests
on Linux

* Create unsigned CAN receive tests

This adds the missing unit tests to cover the receive
mapping of CAN values where the value is larger than
the limit of a signed value. This demonstrates that the
unsigned mapping is working as expected.

The CanMap code has a conditional define to allow
building of libopeninv with signed receive support.
Extend the unit test Makefile to support building with
this and retaining the existing signed receive mapping
tests.

Extend the GitHub Action to build both variants.

Tests:
 - Build with CAN_SIGNED=0 and verify that 32-bit
    receive tests are broken
 - Build with CAN_SIGNED=1 and verify existing tests
    pass

* Fix unsigned CAN receive for 32-bit values

With a receive CAN mapping for a 32-bit value if the
value being received is > 2^31 then it is converted into
a negative number rather than the large positive
number that would be expected. The fix is to avoid
storing the incoming value in a signed type when
building the normal unsigned version of libopeninv.

Tests:
 - Build and run unit tests with CAN_SIGNED=1 and
   CAN_SIGNED=0 with no errors
  • Loading branch information
davefiddes authored Oct 3, 2024
1 parent fe14186 commit 93e9bcd
Show file tree
Hide file tree
Showing 15 changed files with 1,677 additions and 14 deletions.
37 changes: 37 additions & 0 deletions .github/workflows/CI-build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
name: CI
on:
push:
pull_request:

jobs:
build:
name: build-linux
runs-on: ubuntu-latest

steps:
- name: Checkout libopeninv
uses: actions/checkout@v4
with:
path: libopeninv

- name: Checkout libopencm3
uses: actions/checkout@v4
with:
repository: jsphuebner/libopencm3
path: libopencm3

- name: Build unit tests on host
run: |
make -C libopeninv/test clean all CAN_SIGNED=0
- name: Run unit tests on host
run: |
libopeninv/test/test_libopeninv
- name: Build unit tests on host (Signed CAN receive)
run: |
make -C libopeninv/test clean all CAN_SIGNED=1
- name: Run unit tests on host (Signed CAN receive)
run: |
libopeninv/test/test_libopeninv
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*.o
test/test_libopeninv
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
# libopeninv

[![Build status](../../actions/workflows/CI-build.yml/badge.svg)](../../actions/workflows/CI-build.yml)

Generic modules that can be used in many projects
29 changes: 15 additions & 14 deletions src/canmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ void CanMap::HandleRx(uint32_t canId, uint32_t data[2], uint8_t)
{
forEachPosMap(curPos, recvMap)
{
float val;
uint32_t word;
uint8_t pos = curPos->offsetBits;
uint8_t numBits = ABS(curPos->numBits);
Expand Down Expand Up @@ -138,22 +137,24 @@ void CanMap::HandleRx(uint32_t canId, uint32_t data[2], uint8_t)
uint32_t mask = (1L << numBits) - 1;
word = (word >> pos) & mask;

// sign-extend our arbitrary sized integer out to 32-bits but only if
// it is bigger than a single bit
int32_t ival;
#if CAN_SIGNED
if (numBits > 1)
{
uint32_t sign_bit = 1L << (numBits - 1);
ival = static_cast<int32_t>(((word + sign_bit) & mask)) - sign_bit;
}
else
// sign-extend our arbitrary sized integer out to 32-bits but only if
// it is bigger than a single bit
int32_t ival;
if (numBits > 1)
{
uint32_t sign_bit = 1L << (numBits - 1);
ival = static_cast<int32_t>(((word + sign_bit) & mask)) - sign_bit;
}
else
{
ival = word;
}
float val = ival;
#else
float val = word;
#endif
{
ival = word;
}

val = ival;
val += curPos->offset;
val *= curPos->gain;

Expand Down
34 changes: 34 additions & 0 deletions test/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Option to allow signed reception of CAN variables
CAN_SIGNED ?= 0

CC = gcc
CPP = g++
LD = g++
CFLAGS = -std=c99 -ggdb -DSTM32F1 -DCAN_SIGNED=$(CAN_SIGNED) -Itest-include -I../include -I../../libopencm3/include
CPPFLAGS = -ggdb -DSTM32F1 -DCAN_SIGNED=$(CAN_SIGNED) -Itest-include -I../include -I../../libopencm3/include
LDFLAGS = -g
BINARY = test_libopeninv
OBJS = test_main.o fu.o test_fu.o test_fp.o my_fp.o my_string.o params.o \
stub_canhardware.o test_canmap.o canmap.o \
stub_libopencm3.o
VPATH = ../src ../libopeninv/src

# Check if the variable GITHUB_RUN_NUMBER exists. When running on the github actions running, this
# variable is automatically available.
# Create a compiler define with the content of the variable. Or, if it does not exist, use replacement value 99999.
CPPFLAGS += $(shell \
if [ -z "$$GITHUB_RUN_NUMBER" ]; then echo "-DGITHUB_RUN_NUMBER=0"; else echo "-DGITHUB_RUN_NUMBER=$$GITHUB_RUN_NUMBER"; fi )

all: $(BINARY)

$(BINARY): $(OBJS)
$(LD) $(LDFLAGS) -o $(BINARY) $(OBJS)

%.o: ../%.cpp
$(CPP) $(CPPFLAGS) -o $@ -c $<

%.o: ../%.c
$(CC) $(CFLAGS) -o $@ -c $<

clean:
rm -f $(OBJS) $(BINARY)
45 changes: 45 additions & 0 deletions test/stub_canhardware.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* This file is part of the stm32-sine project.
*
* Copyright (C) 2021 Johannes Huebner <[email protected]>
* Copyright (C) 2024 David J. Fiddes <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "stub_canhardware.h"

CanCallback* vcuCan = nullptr;
uint32_t vcuCanId;

CanHardware::CanHardware()
{}

bool CanHardware::AddCallback(CanCallback* cb)
{
vcuCan = cb;
return true;
}

bool CanHardware::RegisterUserMessage(uint32_t canId, uint32_t mask)
{
vcuCanId = canId;
return true;
}

void CanHardware::ClearUserMessages() {}

void CanHardware::HandleRx(uint32_t canId, uint32_t data[2], uint8_t dlc)
{
vcuCan->HandleRx(canId, data, dlc);
}
48 changes: 48 additions & 0 deletions test/stub_canhardware.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* This file is part of the stm32-sine project.
*
* Copyright (C) 2021 Johannes Huebner <[email protected]>
* Copyright (C) 2024 David J. Fiddes <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#ifndef TEST_CANHARDWARE_H
#define TEST_CANHARDWARE_H

#include "canhardware.h"
#include <stdint.h>
#include <string.h>
#include <array>

class CanStub: public CanHardware
{
void SetBaudrate(enum baudrates baudrate) {}
void Send(uint32_t canId, uint32_t data[2], uint8_t len)
{
m_canId = canId;
memcpy(&m_data[0], &data[0], sizeof(m_data));
m_len = len;
}
virtual void ConfigureFilters() {}

public:
std::array<uint8_t, 8> m_data;
uint8_t m_len;
uint32_t m_canId;
};

extern CanCallback* vcuCan;
extern uint32_t vcuCanId;

#endif // TEST_CANHARDWARE_H
58 changes: 58 additions & 0 deletions test/stub_libopencm3.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* This file is part of the stm32-sine project.
*
* Copyright (C) 2024 David J. Fiddes <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "stdint.h"

void flash_unlock(void)
{
}

void flash_lock(void)
{
}

void flash_set_ws(uint32_t ws)
{
}

void flash_program_word(uint32_t address, uint32_t data)
{
}

void flash_erase_page(uint32_t page_address)
{
}

uint16_t desig_get_flash_size(void)
{
return 8;
}

uint32_t crc_calculate(uint32_t data)
{
return 0xaa55;
}

uint32_t crc_calculate_block(uint32_t *datap, int size)
{
return 0xaaaa5555;
}

void crc_reset(void)
{
}
28 changes: 28 additions & 0 deletions test/test-include/hwdefs.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* This file is part of the libopeninv project.
*
* Copyright (C) 2024 David J. Fiddes <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#ifndef HWDEFS_H
#define HWDEFS_H

// Minimal project hardware defines to test libopeninv

#define FLASH_PAGE_SIZE 1024

#define CAN1_BLKNUM 2 // second to last block of 1k

#endif
27 changes: 27 additions & 0 deletions test/test-include/param_prj.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* This file is part of the libopeninv project.
*
* Copyright (C) 2024 David J. Fiddes <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

// Minimal project parameters to test libopeninv
/* category name unit min max default id */
#define PARAM_LIST \
VALUE_ENTRY(amp, "dig", 2013 ) \
VALUE_ENTRY(pot, "dig", 2015 ) \
PARAM_ENTRY("inverter", ocurlim, "A", -65536, 65536, 100, 22 )

extern const char* errorListString;
36 changes: 36 additions & 0 deletions test/test.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#ifndef TEST_H_INCLUDED
#define TEST_H_INCLUDED
#include <iostream>
#include <list>

typedef void (*VoidFunction)();

class UnitTest
{
public:
UnitTest(const std::list<VoidFunction>*);
virtual void TestSetup() {}
virtual void TestCaseSetup() {}
const std::list<VoidFunction> GetCases() { return *_cases; }
void SetTestCaseList(const std::list<VoidFunction>* cases) { _cases = cases; }

private:
const std::list<VoidFunction>* _cases;
};

extern int _failedAssertions;


#define REGISTER_TEST(t, ...) static UnitTest* test = new t (new std::list<VoidFunction> { __VA_ARGS__ });

#define STRING(s) #s
#define ASSERT(c) \
if (c) \
std::cout << "Test " << __FILE__ << "::" << __func__ << " passed." << std::endl; \
else \
{ \
std::cout << "Assertion failed: " << STRING(c) << " in " __FILE__ " : " << std::dec << __LINE__ << std::endl; \
_failedAssertions++; \
}

#endif // TEST_H_INCLUDED
Loading

0 comments on commit 93e9bcd

Please sign in to comment.