PLC logic library

Question

I have been implementing a library of PLC logic blocks (i.e. AND gates, OR gates, RS flip-flops and TON, TOF and TP timers) in C++. I have decided to model all of these logic blocks with C++ classes which implement common interface

namespace LogicBlocks
{

class LogicBlk {
public:
    enum LogicType_e{
        POS, // positive logic
        NEG  // negative logic
    };
    virtual void Update(void) = 0;

private:

};

}

I have already implemented the timers and RS flip-flop logic blocks. What I would like to do now is to implement the AND and OR gates. My idea is to use AND gates and OR gates with 1 input up to 8 inputs. I have implemented this in following manner. I have special class for each type of gate. For example:

OR gate with two inputs

Interface:

namespace LogicBlocks
{

// OR logic gate with two inputs 
class Or_02 : public LogicBlk{
public:
    Or_02(uint32_t *bitsArray, 
          uint32_t input_01, LogicType_e inputType_01, 
          uint32_t input_02, LogicType_e inputType_02, 
          uint32_t output);
    virtual ~Or_02();

    void Update(void);

private:

    uint32_t    m_In01;
    LogicType_e m_In01Type;
    uint32_t    m_In02;
    LogicType_e m_In02Type;
    uint32_t    m_Out;
    uint32_t    *m_BitsArray;

};

}

Implementation:

LogicBlocks::Or_02::Or_02(uint32_t *bitsArray, 
                          uint32_t input_01, LogicType_e inputType_01,
                          uint32_t input_02, LogicType_e inputType_02, 
                          uint32_t output):
        m_BitsArray{bitsArray}, 
        m_In01{input_01}, m_In01Type{inputType_01}, 
        m_In02{input_02}, m_In02Type{inputType_02}, 
        m_Out{output}{
}

LogicBlocks::Or_02::~Or_02() {
}

void LogicBlocks::Or_02::Update(void){

    if(((Utils::TestBitSet(m_BitsArray, m_In01) && m_In01Type == POS) || (Utils::TestBitClr(m_BitsArray, m_In01) && m_In01Type == NEG)) || 
       ((Utils::TestBitSet(m_BitsArray, m_In02) && m_In02Type == POS) || (Utils::TestBitClr(m_BitsArray, m_In02) && m_In02Type == NEG))){
        Utils::SetBit(m_BitsArray, m_Out);
    }else{
        Utils::ClrBit(m_BitsArray, m_Out);
    }
}

OR gate with three inputs

Interface:

namespace LogicBlocks
{

// OR logic gate with three inputs
class Or_03 : public LogicBlk{
public:
    Or_03(uint32_t* const bitsArray, 
          const uint32_t input_01, const LogicType_e inputType_01, 
          const uint32_t input_02, const LogicType_e inputType_02, 
          const uint32_t input_03, const LogicType_e inputType_03, 
          const uint32_t output);
    virtual ~Or_03();

    void Update(void);

private:

    uint32_t    m_In01;
    LogicType_e m_In01Type;
    uint32_t    m_In02;
    LogicType_e m_In02Type;
    uint32_t    m_In03;
    LogicType_e m_In03Type;
    uint32_t    m_Out;
    uint32_t    *m_BitsArray;

};

} 

Implementation:

LogicBlocks::Or_03::Or_03(uint32_t* const bitsArray, 
                          const uint32_t input_01, const LogicType_e inputType_01,
                          const uint32_t input_02, const LogicType_e inputType_02, 
                          const uint32_t input_03, const LogicType_e inputType_03,
                          const uint32_t out):
        m_BitsArray{bitsArray}, 
        m_In01{input_01}, m_In01Type{inputType_01}, 
        m_In02{input_02}, m_In02Type{inputType_02}, 
        m_In03{input_03}, m_In03Type{inputType_03}, 
        m_Out{output}{
}

LogicBlocks::Or_03::~Or_03() {
}

void LogicBlocks::Or_03::Update(void){

    if(((Utils::TestBitSet(m_BitsArray, m_In01) && m_In01Type == POS) || (Utils::TestBitClr(m_BitsArray, m_In01) && m_In01Type == NEG)) || 
       ((Utils::TestBitSet(m_BitsArray, m_In02) && m_In02Type == POS) || (Utils::TestBitClr(m_BitsArray, m_In02) && m_In02Type == NEG)) ||
       ((Utils::TestBitSet(m_BitsArray, m_In03) && m_In03Type == POS) || (Utils::TestBitClr(m_BitsArray, m_In03) && m_In03Type == NEG))){
        Utils::SetBit(m_BitsArray, m_Out);
    }else{
        Utils::ClrBit(m_BitsArray, m_Out);
    }
}

Here is the usage. Lets say I have an object called Logic:

Interface:

#include <stdint.h>
#include "LogicBlk.h"

namespace Logic
{

class Logic{
public:
    Logic();
    virtual ~Logic();

    // method shall be called from a task
    void Loop(void);

private:

    uint32_t m_BitsArray[1] = {0};

    static const uint8_t NO_LOGIC_BLKS = 2;
    LogicBlocks::LogicBlk* m_LogicBlks[NO_LOGIC_BLKS];

};

}

Implementation:

#include "Logic.h"
#include "Bits.h"
#include "Or_02.h"
#include "And_02.h"

    #define LW_01       (0)

    // Byte 01         
    #define LSig01      (LW_01*32 + 0x00)   
    #define LSig02      (LW_01*32 + 0x01)   
    #define LSig03      (LW_01*32 + 0x02)
    //efine L           (LW_01*32 + 0x03)
    //efine L           (LW_01*32 + 0x04)
    //efine L           (LW_01*32 + 0x05)
    //efine L           (LW_01*32 + 0x06)
    //efine L           (LW_01*32 + 0x07)

    // Byte 02
    #define LAx01       (LW_01*32 + 0x08)
    #define LAx02       (LW_01*32 + 0x09)
    //efine L           (LW_01*32 + 0x0A)
    //efine L           (LW_01*32 + 0x0B)
    //efine L           (LW_01*32 + 0x0C)
    //efine L           (LW_01*32 + 0x0D)
    //efine L           (LW_01*32 + 0x0E)
    //efine L           (LW_01*32 + 0x0F)

    // Byte 03
    //efine L           (LW_01*32 + 0x10)
    //efine L           (LW_01*32 + 0x11)
    //efine L           (LW_01*32 + 0x12)
    //efine L           (LW_01*32 + 0x13)
    //efine L           (LW_01*32 + 0x14)
    //efine L           (LW_01*32 + 0x15)
    //efine L           (LW_01*32 + 0x16)
    //efine L           (LW_01*32 + 0x17)

    // Byte 04
    //efine L           (LW_01*32 + 0x18)
    //efine L           (LW_01*32 + 0x19)
    //efine L           (LW_01*32 + 0x1A)
    //efine L           (LW_01*32 + 0x1B)
    //efine L           (LW_01*32 + 0x1C)
    //efine L           (LW_01*32 + 0x1D)
    //efine L           (LW_01*32 + 0x1E)
    //efine L           (LW_01*32 + 0x1F)

Logic::Logic::Logic(){

    m_LogicBlks[0] = new LogicBlocks::Or_02(m_BitsArray,
                                            LSig01,       LogicBlocks::LogicBlk::POS,
                                            LSig02,       LogicBlocks::LogicBlk::POS,
                                            LAx01);
    m_LogicBlks[1] = new LogicBlocks::And_02(m_BitsArray,
                                             LAx01,      LogicBlocks::LogicBlk::POS,
                                             LSig03,     LogicBlocks::LogicBlk::NEG,
                                             LAx02);

}

Logic::Logic::~Logic(){
}

void Logic::Logic::Loop(void){

    for(uint8_t curBlk = 0; curBlk < NO_LOGIC_BLKS; curBlk++){
        m_LogicBlks[curBlk]->Update();
    }

}

For completeness the utility functions for work with individual bits

bool Utils::TestBitSet(uint32_t *bitsArray, uint32_t bit){
    uint32_t wordValue    = *(bitsArray + (bit >> 5));
    uint32_t bitPosInWord = (bit - ((bit >> 5) << 5));

    return ((wordValue & ((uint32_t)1 << bitPosInWord)) >> bitPosInWord) ? true : false;
}

bool Utils::TestBitClr(uint32_t *bitsArray, uint32_t bit){
    uint32_t wordValue    = *(bitsArray + (bit >> 5));
    uint32_t bitPosInWord = (bit - ((bit >> 5) << 5));

    return ((wordValue & ((uint32_t)1 << bitPosInWord)) >> bitPosInWord) ? false : true; 
}

void Utils::SetBit(uint32_t *bitsArray, uint32_t bit){
    uint32_t word         = (bit >> 5);
    uint32_t bitPosInWord = (bit - ((bit >> 5) << 5));

    *(bitsArray + word) |= ((uint32_t)1 << bitPosInWord);
}

void Utils::ClrBit(uint32_t *bitsArray, uint32_t bit){
    uint32_t word         = (bit >> 5);
    uint32_t bitPosInWord = (bit - ((bit >> 5) << 5));

    *(bitsArray + word) &= ~((uint32_t)1 << bitPosInWord);
}

void Utils::NegBit(uint32_t *bitsArray, uint32_t bit){
    if(TestBitSet(bitsArray, bit)){
        ClrBit(bitsArray, bit);
    }else{
        SetBit(bitsArray, bit);
    }
}

Now I have been thinking about weaknesses of this implementation. First of all I thing that implementation of OR gate with three inputs is sort of code repetition of OR gate with two inputs. Exactly the same is true for OR gates with more inputs. The second problem which I have is the way how the gates "communicate" and form the whole logic. Here I have been using bits array. I have also took into account a possibility that each logic block would receive pointers to cooperating logic blocks. The reason why I didn't use this approach was that in my opinion this implementation would lead to tree data structure and in case evaluation of the whole logic the stack can grow rapidly especially in complex logic structures.

Does anybody have any idea how to resolve my issues i.e. code repetition and communication between logic blocks? Thank you for any suggestions.

Answer 1

Parameterize Logic Type

I'd start by trying to consolidate the mapping from voltage level to logic level in one place. For example:

class positive_signal { 
    unsigned value : 1;
public:
     bool isSet() const { return value == 1; }
     void set() { value = 1; }
     void clear() { value = 0; }
};

class negative_signal {
    unsigned value : 1;
public:
    bool isSet() const { return value == 0; }
    void set() { value = 0; }
    void clear() { value = 1; }
};

Note that I'm leaving out a lot of detail here, just giving a sketch of a general direction. Just for example, in practice there's a decent chance that you'd want the set and clear member functions to return *this; to support chaining.

Generalize Gates

I'd consider implementing each gate type to take an arbitrary number of inputs:

namespace logic {

    bool OR(std::vector<signal> const &inputs) { 
        return std::any_of(inputs().begin(), inputs.end(),
                           [](signal in) { return in.isSet; });
    }

    bool AND(std::vector<signal> const &inputs) { 
        return std::all_of(inputs.begin(), inputs.end(), 
                           [](signal in) { return in.isSet; });
    }
    // and so on
}

This avoids duplicating logic for gates with different numbers of inputs, because you use exactly the same code for both.

Integration

Putting those two together can be a little tricky though. As I've shown the code above, they don't really fit well. You can go a couple of different routes. One is to use inheritance, so you'd start with signal as an abstract base class, and then positive and negative signals derived from that. If you do that, you'd have to pass vectors of pointers to signals, rather than vectors of signals.

Alternatively, you could pass the signal type as a template parameter to the gate, so the compiler would instantiate one OR for negative logic and a separate OR for positive logic (and so on).

In this case, I think the latter is a better fit. Inheritance would make sense if you expected to create something like a 5-input OR gate, with 2 arbitrary inputs being negative logic, and the other three positive logic (or more generally, inputs could be any arbitrary combination of positive/negative logic). In reality, however, you typically define the logic type at the gate level, so a 5-input OR gate is going to take either all 5 inputs as positive logic, or else all 5 inputs as negative logic, but never some combination of the two.

Answer 2

LogicBlocks::Or_02::Or_02(uint32_t *bitsArray, 
                          uint32_t input_01, LogicType_e inputType_01,
                          uint32_t input_02, LogicType_e inputType_02, 
                          uint32_t output):
        m_BitsArray{bitsArray}, 
        m_In01{input_01}, m_In01Type{inputType_01}, 
        m_In02{input_02}, m_In02Type{inputType_02}, 
        m_Out{output}{
}

Member variables are initialized in the order specified in the class, not the order in the initializer list (so m_BitsArray will be initialized last). While nothing will break here, it's best to always use the correct order in the initializer list.

---

(Utils::TestBitSet(m_BitsArray, m_In01) && m_In01Type == POS)

There's a lot of code like this. Why not factor it into a function taking all three variables:

Utils::TestBitSet(m_BitsArray, m_In01, m_In01Type);

---

[Opinion] I'm not too fond of the separate TestBitSet() and TestBitClr(). It's neater to just have one IsBitSet() and use !IsBitSet() where appropriate.

---

new LogicBlocks::Or_02

Don't just leak memory. Use a std::unique_ptr, or delete it manually.

---

LogicBlk must have a virtual destructor (and then you also don't need to specify empty destructors in the derived classes).

---

virtual Update(void);

The void parameter isn't necessary in C++.

---

#define LSig01      (LW_01*32 + 0x00)
...

Prefer constant static variables to #defines, because they have proper scoping.

---

Or_03(uint32_t* const bitsArray, 
      const uint32_t input_01, const LogicType_e inputType_01, ...

Don't make function arguments that are passed by value const. These consts don't actually matter to the caller, and they hide the consts that do matter (e.g. & vs const& or * vs const*), which makes the declaration harder to read. C++ also allows the use of const in a declaration to be different from the use of const in the function definition, so it can even be misleading.

---

We can use templates for the different numbers of inputs:

struct LogicInput {
    LogicType_e type;
    std::uint32_t value;
};

template<std::size_t NumInputs>
class Or {
public:

    Or(uint32_t* bitsArray, std::array<LogicInput>, NumInputs> in, uint32_t out);

    ...

    std::array<LogicInput> m_In;
    std::uint32_t m_Out;
    uint32_t* m_BitsArray;
};