Implementing PLC "Timer Pulse" function block in C++ I have been implementing a library of the PLC function blocks. One of the function blocks is the "Timer Pulse" function block which behavior is following:
 

I have implemented this function block in this manner

Tpulse.h:

#include &lt;stdint.h&gt;
#include "LogicBlk.h"

namespace LogicBlocks
{

// Timer Pulse i.e. logic one at the output for specified time as soon as 0-&gt;1 at the input
class Tpulse : public LogicBlk{
public:
    Tpulse(uint32_t*, uint32_t, uint32_t, uint32_t);
    virtual ~Tpulse();

    void Update(void);

private:

    uint32_t *m_BitsArray;
    uint32_t m_Input;
    uint32_t m_Output;
    uint32_t m_PulseTime;
    uint32_t m_StartTime;
    uint32_t m_ElapsedTime;
    uint32_t m_CurrentTime;

};

}


Tpulse.cpp:

LogicBlocks::Tpulse::Tpulse(uint32_t *bitsArray, uint32_t input, uint32_t output, uint32_t pulseTime):
                            m_BitsArray{bitsArray}, m_Input{input}, m_Output{output}, m_PulseTime{pulseTime}{
                                m_StartTime     = 0;
                                m_ElapsedTime   = 0;
                                m_CurrentTime   = 0;
}

LogicBlocks::Tpulse::~Tpulse(){
}

void LogicBlocks::Tpulse::Update(void){

    if(Utils::TestBitSet(m_BitsArray, m_Input) &amp;&amp; Utils::TestBitClr(m_BitsArray, m_Output) &amp;&amp; !m_ElapsedTime){

        m_StartTime = GetTick();
        SetBit(m_BitsArray, m_Output);

    }else if(Utils::TestBitSet(m_BitsArray, m_Output)){

        m_CurrentTime = GetTick();
        m_ElapsedTime = m_CurrentTime - m_StartTime;

        if(m_ElapsedTime &gt;= m_PulseTime){

            ClrBit(m_BitsArray, m_Output);
            if(TestBitClr(m_BitsArray, m_Input)){

                m_ElapsedTime = 0;
            }
        }

    }else if(TestBitClr(m_BitsArray, m_Input) &amp;&amp; TestBitClr(m_BitsArray, m_Output)){

        m_ElapsedTime = 0;

    }

}


LogicBlk.h

namespace LogicBlocks
{

class LogicBlk {
public:

    virtual void Update(void) = 0;

private:

};

}


Based on the tests which I have already done it seems to me that it works fine but I am not sure. Please can anybody assess my code from functional and programming style point of view? Thank you in advance for any suggestions.
I see a number of things that may help you improve your code.

Let the compiler write default functions

The Tpulse destructor literally does nothing and is what the compiler would have generated anyway.  To indicate that, eliminate the function in Tpulse.cpp and declare it as default in Tpulse.h:

virtual ~Tpulse() = default;


If you define one special member function, define them all

Sometimes called the "rule of five," if you delete or define any of the special member functions, you should delete or define them all. See C.21

Use override where appropriate

If you're intending to override a virtual base function, you should explicitly say so to help catch errors.  In this case it's the Update function which should be marked override:

void Update() override;


See C.128  Also note that unlike in C, Update(void) and Update() mean the same thing in C++.  I prefer the shorter form, but sometimes coding guidelines (often written by or for C programmers) require the long form.

Use namespaces consistently

In some places within Tpulse::Update, TestBitClr() is written with a Utils:: namespace prefix and in other place not.  This is inconsistent and confusing to the reader who may be left wondering if there are two versions.  Instead, I'd recommend either always using the explicit namespace or putting a using namespace Util; within the Update function.

Prefer std:: namespace versions of functions and types

Rather than uint32_t, I'd recommend using std::uint32_t and explicitly adding #include &lt;cstdint&gt;.  This makes it clear which type you mean and will be robust even if someone later introduces a local uint32_t which does, unfortunately, sometimes happen in embedded systems projects.

Use parameter names in function templates

It's best to make the interface clear and explicit, and with a function call like this:

Tpulse(uint32_t*, uint32_t, uint32_t, uint32_t);


It's not clear what the various uint32_t values represent.  If they were written like this, it would be better:

Tpulse(std::uint32_t *bitsArray, std::uint32_t input, std::uint32_t output, std::uint32_t pulseTime):


See I.4.

Think about the possible correct use of volatile

In embedded systems we often encounter one of the few correct uses of volatile.  In this case, I'm wondering about the m_BitsArray pointer.  If, as I suspect, this is memory-mapped I/O, then this "memory" is not really solely under the control of the C++ environment since external asynchronous signals may cause those bits to change outside program control.  For that reason, it may be that it should be declared as volatile to indicate this fact.  See CP.200.  On the other hand, if this array might also be use by other threads within your program, you will need to add explicit synchronization of some kind.  See CP.8

Use const where practical

It appears to me that m_Input and m_Output should never change within the lifetime of a Tpulse object, so I'd suggest that both of those member data functions should be declared const.  The same is probably true of m_PulseTime.

Minimize the interface

It seems to me that the Tpulse class could eliminate m_ElapsedTime and m_CurrentTime in favor of only using m_StartTime and perhaps a bool running.  If you need other named variables within Update they can be local.

Use helper functions

The code could be much simpler to read and understand with the use of a few private helper functions:

bool LogicBlocks::Tpulse::input() const {
    return Utils::TestBitSet(m_BitsArray, m_Input);
}

bool LogicBlocks::Tpulse::output(bool value) {
    if (value) {
        Utils::SetBit(m_BitsArray, m_Output);
    } else {
        Utils::ClrBit(m_BitsArray, m_Output);
    }
    return value;
}


Simplify the code

The Update code is a little more complex than it needs to be.  Essentially, either the timer is running and we do timer things, or it's not yet running, but we receive an input that tells us to start it.  The only other slightly tricky thing is that we don't allow the timer to restart until it's expired and the input is low.

So we can simplify the code, using a bool running member variable and the helper functions shown above:

void LogicBlocks::Tpulse::Update(){
    if (running) {  
        auto elapsed = Utils::GetTick() - m_StartTime;
        if (elapsed &gt;= m_PulseTime) {
            output(false);
            running = input();
        }
    } else if (input()) {
        m_StartTime = Utils::GetTick();
        running = output(true);
    } 
}


Provide complete code to reviewers

This is not so much a change to the code as a change in how you present it to other people.  Without the full context of the code and an example of how to use it, it takes more effort for other people to understand your code.  This affects not only code reviews, but also maintenance of the code in the future, by you or by others.  One good way to address that is by the use of comments.  Another good technique is to include test code showing how your code is intended to be used.

In this case I wrote this main to do some testing:

#include "Tpulse.h"
#include "Utils.h"
#include &lt;iostream&gt;
#include &lt;iomanip&gt;
#include &lt;chrono&gt;
#include &lt;thread&gt;

int main() {
    using namespace Utils;
    using namespace std::chrono_literals;
    uint32_t reg{0};
    constexpr uint32_t inbit{0x80};
    constexpr uint32_t outbit{0x2};
    LogicBlocks::Tpulse tp(&amp;reg, inbit, outbit, 5);
    for (int i=0; i &lt; 20; ++i) {
        std::this_thread::sleep_for(1s);
        if (i == 2 || i == 9) {
            SetBit(&amp;reg, inbit);
        } else if (i == 4 || i == 16) {
            ClrBit(&amp;reg, inbit);
        }
        std::cout &lt;&lt; std::dec &lt;&lt; "t = " &lt;&lt; i &lt;&lt; ", reg = 0x" &lt;&lt; std::hex &lt;&lt; reg;
        tp.Update();
        std::cout &lt;&lt; ", updated to 0x" &lt;&lt; reg &lt;&lt; '\n';
    }
}


Here's the output:

t = 0, reg = 0x0, updated to 0x0
t = 1, reg = 0x0, updated to 0x0
t = 2, reg = 0x80, updated to 0x82
t = 3, reg = 0x82, updated to 0x82
t = 4, reg = 0x2, updated to 0x2
t = 5, reg = 0x2, updated to 0x2
t = 6, reg = 0x2, updated to 0x2
t = 7, reg = 0x2, updated to 0x0
t = 8, reg = 0x0, updated to 0x0
t = 9, reg = 0x80, updated to 0x82
t = 10, reg = 0x82, updated to 0x82
t = 11, reg = 0x82, updated to 0x82
t = 12, reg = 0x82, updated to 0x82
t = 13, reg = 0x82, updated to 0x82
t = 14, reg = 0x82, updated to 0x80
t = 15, reg = 0x80, updated to 0x80
t = 16, reg = 0x0, updated to 0x0
t = 17, reg = 0x0, updated to 0x0
t = 18, reg = 0x0, updated to 0x0
t = 19, reg = 0x0, updated to 0x0