Bipedal-locomotion-framework: Discussing on the possibility to implement a generalized block class

[GiulioRomualdi]

I notice that we are implementing the initialize() method for each class (i.e. here or here) and in general the class cannot be used till the initialize() method is called.
Probably we can generalize the concept by designing a class named block (or another name). The other classes will inherit from block.

This is an example of the class

class Block
{
protected:
    enum class State
    {
         NotInitialized, Initialized, ...
    };
   State m_state;
public:
   virtual bool initialize(std::weak_ptr<IParameterHandler> handler) {return true;};
   void bool close() {return true;};
   virtual ~Block();
};

This issue is for discussing the possible strategies we can implement.
@traversaro @S-Dafarra @MiladShafiee @prashanthr05

[S-Dafarra]

I agree.
In general, I think it is better not to rely on the user of the interface to change a flag in the base class. As an example, if you want to know the state of a block, I would do

class Block
{
public:
    enum class State
    {
         NotInitialized, Initialized, ...
    };
private:
   State m_state;
protected:
    virtual bool customInitialization(std::weak_ptr<IParameterHandler> handler) {return true;};
public:
   bool initialize(std::weak_ptr<IParameterHandler> handler) 
   {
       m_state = State::NotInitialized;
       if (customInitialization(handler))
       {
            m_state = State::Initialized;
           return true;
        }
        return false;
   };
   State getState() const;
   void bool close() {return true;};
   virtual ~Block();
};

[GiulioRomualdi]

I think it is better not to rely on the user of the interface to change a flag in the base class.

I agree on that, however, the user has to check the status of the flag and they may modified it
i.e. switching from State::Initialized to another state

[S-Dafarra]

I agree on that, however, the user has to check the status of the flag and they may modified it
i.e. switching from State::Initialized to another state

That's why I put m_state private and added the getState() method. Anyway, this is only in case you want to keep track of the state in the base class, or you need to access it from the outside having a pointer to the base class. Otherwise, you can avoid defining the state at all, leaving to the user the freedom to define the state.

Also, in my opinion, if you define a specific meaning for a state, you should not let the user the ability to change it freely. There is quite a chance to introduce bugs.

[GiulioRomualdi]

A block may also contain a shared_ptr to MatLogger2 and a virtual function call log(). I.e.

virtual bool log(){return true};

This may be really useful for debugging purpose.

[traversaro]

General comment: I would strictly separate interfaces from class that have concrete methods and attributes. In my experience mixing the two concepts creates problems in the long run (sorry for not being verbose, if you are interested we can discuss more in depth).

[GiulioRomualdi]

Thanks for the hint!

[prashanthr05]

I propose the block states as,

• NotConfigured, the required parameters for the block is not setup
• Configured, the required parameters for the block is set up. Once configured, we can set inputs to the block.
• NotInitialized, the block is not initialized (for example, a pending calibration procedure is yet to finish, this procedure uses the incoming inputs already at this step, so setInputs() and advance() can be called)
• Initialized, the block is initialized and can now get into an expected functionality step
• Running, the block is running

Additionally, we can also have
- DynamicReconfigure, to reset the parameters of the block (eg. http://wiki.ros.org/dynamic_reconfigure)

[S-Dafarra]

@prashanthr05 if you have already some use case in mind, feel free to open a PR.