lexus2k / tinyproto

Tiny Software Protocol for communication over UART, SPI, etc
GNU General Public License v3.0
224 stars 49 forks source link

Unittests don't compile #45

Open bakeromso opened 3 days ago

bakeromso commented 3 days ago

Thanks for the extensive protocol you've made! I'm trying it out, and wanted to run the unit tests, but I'm running into compile issues. I'm building with a container, but the same issue also arises bare metal.

Reproducing the issue

If you're interested in reproducing the issue, this is my setup

Dockerfile:

# Use official Ubuntu rolling base image
FROM ubuntu:24.04

# Install gcc, g++, and make
RUN apt-get update && apt-get install -y \
    gcc \
    g++ \
    make \
    cmake \
    pkg-config 

RUN apt-get update && apt-get install -y \
    git

# Since the packed Cpputest is problematic for Ubuntu, let's build it ourselves
RUN cd /root && \
    git clone https://github.com/cpputest/cpputest.git && \
    cd cpputest && \ 
    mkdir cpputest_build && \ 
    cmake -S . -B cpputest_build && \ 
    cd cpputest_build && \
    make install 

CMD [ "/bin/bash" ]

build it:

docker build . -t tinyproto

And run it (from this repo root):

docker run -v "${PWD}:/root/tinyproto:rw" -w /root/tinyproto -t  tinyproto

From the docker:

cmake -S . -B ./build -DEXAMPLES=ON -DUNITTESTS=ON
cd build
make

Compilation problems

The build fails with the following output:

[ 52%] Built target tinyproto
[ 56%] Built target tiny_loopback
[ 61%] Built target hdlc_demo
[ 65%] Built target hdlc_demo_multithread
[ 68%] Building CXX object unittest/CMakeFiles/unit_test.dir/fd_multidrop_tests.cpp.o
In file included from /usr/include/c++/13/functional:59,
                 from /root/tinyproto/unittest/fd_multidrop_tests.cpp:29:
/usr/include/c++/13/bits/std_function.h: In static member function 'static void std::_Function_base::_Base_manager<_Functor>::_M_create(std::_Any_data&, _Fn&&, std::true_type)':
/usr/include/c++/13/bits/std_function.h:152:20: error: '__dest' does not name a type; did you mean '__dev_t'?
  152 |             ::new (__dest._M_access()) _Functor(std::forward<_Fn>(__f));
      |                    ^~~~~~
      |                    __dev_t
/usr/include/c++/13/bits/std_function.h:152:26: error: expected ')' before '.' token
  152 |             ::new (__dest._M_access()) _Functor(std::forward<_Fn>(__f));
      |                          ^
/usr/include/c++/13/bits/std_function.h:152:19: note: to match this '('
  152 |             ::new (__dest._M_access()) _Functor(std::forward<_Fn>(__f));
      |                   ^
make[2]: *** [unittest/CMakeFiles/unit_test.dir/build.make:76: unittest/CMakeFiles/unit_test.dir/fd_multidrop_tests.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:259: unittest/CMakeFiles/unit_test.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

This seems similar to this issue in cpputest, but their they proposed the a fix of moving the system includes before the cputest include headers. However, the first file to fails the compilation (fd_multidrop_tests.cpp) is already failing on the first include. This include is a system include rather than a cpputest include, so there is nothing that could be moved further up here. In other words, I don't see how the proposed fix of that issue would apply here.

I have no experience with cpputest, would you have any idea what's going on here? Are the unit tests still compiling on your system?

bakeromso commented 3 days ago

Edit:

I managed to get them to compile by disabling cpputest's memoryleak detection, dockerfile:

# Use official Ubuntu rolling base image
FROM ubuntu:24.04

# Install gcc, g++, and make
RUN apt-get update && apt-get install -y \
    gcc \
    g++ \
    make \
    cmake \
    pkg-config 

RUN apt-get update && apt-get install -y \
    git

# Since the packed Cpputest is problematic for Ubuntu, let's build it ourselves
RUN cd /root && \
    git clone https://github.com/cpputest/cpputest.git && \
    cd cpputest && \ 
    mkdir cpputest_build && \ 
    cmake -S . -B cpputest_build -DCPPUTEST_MEM_LEAK_DETECTION_DISABLED=1 && \ 
    cd cpputest_build && \
    make install 

CMD [ "/bin/bash" ]

And in the unittests cmakelists.txt enforce the use of C++20 standard:

set(CMAKE_CXX_STANDARD 20)  # Replace 11 with 14 or 17 as needed
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)  # Keep this OFF to stick to the standard without compiler-specific extensions

If you like, I can make a PR out of this with the fixes. If this doesn't make any sense, please let me know as well!