tomstewart89 / BasicLinearAlgebra

A library for using matrices and linear algebra on Arduino
MIT License
185 stars 38 forks source link

Error using CMake -- multiple definitions of "Serial" #68

Closed seqwalt closed 11 months ago

seqwalt commented 12 months ago

When trying to make a c++ class that utilizes BLA, I encounter an error when testing on Linux and Mac. After running cmake .. and make from directory Test_BLA/build/, I receive the error:

Scanning dependencies of target TEST
[ 25%] Building CXX object CMakeFiles/TEST.dir/Test.cpp.o
[ 50%] Linking CXX static library libTEST.a
[ 50%] Built target TEST
Scanning dependencies of target my_test
[ 75%] Building CXX object CMakeFiles/my_test.dir/main.cpp.o
[100%] Linking CXX executable my_test
/usr/bin/ld: libTEST.a(Test.cpp.o):(.bss+0x0): multiple definition of `Serial'; CMakeFiles/my_test.dir/main.cpp.o:(.bss+0x0): first defined here
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/my_test.dir/build.make:85: my_test] Error 1
make[1]: *** [CMakeFiles/Makefile2:78: CMakeFiles/my_test.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

Here is my directory structure: |├── BasicLinearAlgebra/ |└── Test_BLA/ | . . . ├── Arduino.h | . . . ├── build/ | . . . ├── CMakeLists.txt | . . . ├── main.cpp | . . . ├── Test.cpp | . . . └── Test.h

The contents of Arduino.h are the same as https://github.com/tomstewart89/BasicLinearAlgebra/blob/master/test/Arduino.h.

Contents of CMakeLists.txt:

cmake_minimum_required(VERSION 3.2)

project(my_test_proj)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17 -O3")

# Test lib
add_library(TEST Test.cpp)

# Arduino BasicLinearAlgebra
add_library(BLA INTERFACE)
target_include_directories(BLA INTERFACE ${CMAKE_SOURCE_DIR}/../BasicLinearAlgebra)

# Executable
add_executable(my_test main.cpp)
include_directories(
    ${CMAKE_SOURCE_DIR}/../BasicLinearAlgebra
    ${PROJECT_SOURCE_DIR}
)
target_link_libraries(my_test
    BLA
    TEST
)

Contents of main.cpp:

#include "Test.h"
#include <BasicLinearAlgebra.h>

int main(){
    Test test;
}

Contents of Test.h:

#pragma once

#include <BasicLinearAlgebra.h>

class Test
{
public:
  Test();
};

Contents of Test.cpp:

#include "Test.h"

Test::Test()
{
}

Note that if I change the instance name of the Print struct within Arduino.h to be Serial_test instead of Serial, then the error message changes to multiple definition of `Serial_test'. Thanks for any help and this great library!

tomstewart89 commented 11 months ago

Hey @seqwalt , thanks for the question and sorry about the slow reply!

The problem is that I was a bit lazy when I wrote Arduino.h and I instantiated Serial in that header file. That means for your setup it'll be defined twice, once when Test.h includes BasicLinearAlgebra.h and again in the TEST library.

For the time being the easiest (and slightly hacky) way to fix this would be to just move the definition of Serial from here into say, main.cpp.

When I get a chance I'll push a change to fix this properly too. Hope that helps! Let me know if you have any more trouble

seqwalt commented 11 months ago

Hey @tomstewart89 no worries on the slow reply!

Thanks for the explanation, I have a quick follow up question. Following your advice, I moved the definition of Serial into main.cpp, such that main.cpp looks like:

#include "Test.h"
#include <BasicLinearAlgebra.h>

int main(){
    Test test;
    Print Serial;
    BLA::Matrix<3, 3> A;
    A.Fill(0);
    Serial << A;
}

and Arduino.h looks like:

#pragma once

#include <algorithm>
#include <iomanip>
#include <sstream>

struct Print
{
    // stuff in the struct
    ...
};

using std::endl;
using std::max;

It compiles, however when I run ./my_test, nothing is printed to the terminal. Is this the expected behavior? Thanks again for all the help :)

tomstewart89 commented 11 months ago

Hey @seqwalt, confusingly, that actually is the intended behavior. I wrote that Serial implementation just to help with my unit tests, not for printing to the console. If you want to see what's being written to that Serial you can write:

std::cout << Serial.buf.str();

You can also just define this function somewhere in your project:

template <typename DerivedType, int Rows, int Cols, typename DType>
std::ostream &operator<<(std::ostream &strm, const MatrixBase<DerivedType, Rows, Cols, DType> &mat)
{
    strm << '[';

    for (int i = 0; i < Rows; ++i)
    {
        strm << '[';

        for (int j = 0; j < Cols; ++j)
        {
            strm << mat(i, j) << ((j == Cols - 1) ? ']' : ',');
        }

        strm << (i == Rows - 1 ? ']' : ',');
    }
    return strm;
}

And then you'll be able to print matrices to std::cout directly like so:

Matrix<3, 3> A = {3., 5., 8., 4., 7., 9., 2., 5.0, 10.};
std::cout << A;

Hope that helps!

seqwalt commented 11 months ago

Thanks @tomstewart89 this is very helpful!