Multiple FSM's in same program with colliding state names have unexpected side effects

[taliesin]

Most probably I'm missing something trivial here, but I'm unable to understand what is happening in the following case:

1. I have 2 files 'wifi_sta_manager.cpp' and 'wifi_ap_manager.cpp'
2. Both hold a 'local' tinyFSM state machine, one is WiFiStationFSM, the other one is WiFiSoftAPFSM
3. Some of the states of these FSMs have the same names: Idle, Started ...
4. Both are initialized using FSM_INITIAL_STATE(name of FSM, Idle)

This is the station FSM

// states forward declaration
struct Idle;
struct Started;
struct Connected;
struct BackOff;

// State Machine Base Class
struct WiFiStationFSM : tinyfsm::Fsm<WiFiStationFSM>
{
    virtual void react(const tinyfsm::Event) {}

    virtual void react(const WiFiStarted &) {}
    virtual void react(const WiFiConnected &) {}
    virtual void react(const WiFiDisconnected &) {}
    // default behaviour is to stop wifi on transition to Idle
    virtual void react(const SetttingsChanged &) { transit<Idle>(station_stop); }
    virtual void react(const OneSecond &) {}

    virtual void entry() {}
    virtual void exit() {}
};
// ... state implementation to follow

// Initial state definition
FSM_INITIAL_STATE(WiFiStationFSM, Idle)

// and somewhere
WiFiStationFSM::start();

And again for the AP FSM:

// states forward declaration
struct Idle;
struct Started;

// State Machine Base Class
struct WiFiSoftAPFSM : tinyfsm::Fsm<WiFiSoftAPFSM>
{
    virtual void react(const tinyfsm::Event) {}

    virtual void react(const WiFiStarted &) {}
    // default behaviour is to stop wifi on transition to Idle
    virtual void react(const SetttingsChanged &) { transit<Idle>(softap_stop); }

    virtual void entry() {}
    virtual void exit() {}
};

// Initial state definition
FSM_INITIAL_STATE(WiFiSoftAPFSM, Idle)

// and somewhere
WiFiSoftAPFSM::start();

The trouble is, that the AP FSM start does not enter the Idle state. If I rename the states to APIdle and APStarted it works.
None of the state structs are defined in a header file, but solely in 'their' cpp files.

What am I missing?
BTW: This is an ESP32 project (IDF 5.2.1), riscv32-esp-elf-gcc (crosstool-NG esp-13.2.0_20230928) 13.2.0

[digint]

The point is that under the hood TinyFSM will instantiate a _state_instance<Idle> template [1] in FSM_INITIAL_STATE [2]. I don't have all details in my Head any more, but I believe that template instantiation is (or can be) done at link time (see comment [3][4] and "implicit instantiation" [5]), which means that the linker finds two declarations of struct Idle and somehow mixes them up. What surprises me a bit is that there is no warning / error when this happens (is there?).
I believe I played around with cases like this back when I wrote TinyFSM, but not sure any more.

As a workaround, try to put the code in each cpp-file into different namespaces, this might help.

Another fix would be to make templates out of your state classes similar to the "multiple_switch" example [6], but this leads to very unreadable code and thus is kind-of discouraged.

[1] https://github.com/digint/tinyfsm/blob/v0.3.3/include/tinyfsm.hpp#L71-L77
[2] https://github.com/digint/tinyfsm/blob/v0.3.3/include/tinyfsm.hpp#L247
[3] https://github.com/digint/tinyfsm/blob/v0.3.3/doc/40-Usage.md?plain=1#L98-L100
[4] https://github.com/digint/tinyfsm/blob/v0.3.3/examples/elevator/Makefile#L73-L75
[5] https://en.cppreference.com/w/cpp/language/class_template
[6] https://github.com/digint/tinyfsm/blob/v0.3.3/examples/api/multiple_switch.cpp

Ultimately it would be nice to have some simple example code to illustrate this problem on "vanilla gcc", after all this might be just some bug in your compiler suite; risc-v ftw, but the last time I played around with ESP32 (a couple of years ago) I abandoned it as everything in their ecosystem was closed source and badly documented...
Sadly I'm very busy at the moment, not much time to test/debug this right now, and to have some fun with risc-v :(

[taliesin]

Ok, repeated it with my host gcc:

$ gcc --version
gcc (Ubuntu 13.2.0-23ubuntu4) 13.2.0
$ g++ -Wall main.cpp one.cpp two.cpp -o onetwo

main.cpp:

extern void init_one();
extern void init_two();

int main()
{
        init_one();
        init_two();
}

one.cpp

#include <iostream>

#include "tinyfsm.hpp"

struct One : tinyfsm::Fsm<One>
{
        virtual void entry() {}
};

struct FirstState : One
{
        void entry() override { std::cout << "entering Firstate of One" << std::endl; }
};

FSM_INITIAL_STATE(One, FirstState);

void init_one()
{
        One::start();
}

two.cpp

#include <iostream>

#include "tinyfsm.hpp"

struct Two : tinyfsm::Fsm<Two>
{
        virtual void entry() {}
};

struct FirstState : Two
{
        void entry() override { std::cout << "entering Firstate of Two" << std::endl; }
};

FSM_INITIAL_STATE(Two, FirstState);

void init_two()
{
        Two::start();
}

Output is:

$ ./onetwo
entering Firstate of One
entering Firstate of One

There is no warning here as-well. So it doesn't seem like a compiler issue.

[taliesin]

And placing it in separate namespaces, like:

namespace Two
{
    struct Two : tinyfsm::Fsm<Two> {  virtual void entry() {} };
    struct FirstState : Two { void entry() override { std::cout << "entering Firstate of Two" << std::endl; } };
};

FSM_INITIAL_STATE(Two::Two, Two::FirstState);

void init_two()
{
        Two::Two::start();
}

... fixes it, which was to be expected.

[digint]

The problem is that you are violating the "One Definition Rule" (ODR) [1]. There is a -Wodr flag for this in gcc, but it seems to only trigger along with -flto.

[1] https://en.wikipedia.org/wiki/One_Definition_Rule

In order to demonstrate this, I have added your example in the "one-definition-rule" branch:

# gcc --version
gcc (Gentoo 13.2.1_p20240210 p14) 13.2.1 20240210

# git checkout one-definition-rule
# cd examples/violate-odr

# make
[no errors, no warnings]
# ./odr
entering Firstate of One
entering Firstate of One

# make clean
# FLAGS=-flto make
two.cpp:10:8: warning: type 'struct FirstState' violates the C++ One Definition Rule [-Wodr]
   10 | struct FirstState : Two
      |        ^

So this is either a gcc bug, or for some reasons it is not possible to resolve ODR problems without LTO.

Anyways I should place some warning in TinyFSM regarding this problem.

[taliesin]

OK, thanks for the clarification, I didn't know about ODR.

I wonder if the FSM_INITIAL_STATE macro could explicitly 'associate' the passed state as being derived from the passed FSM and therefore hint template deduction? I'm not too deep into templates, though.