Template parameter with variadics and shared_ptr

[cmisip]

How come the commented out code does not work?. I am trying to come up with a way to pass multiple arguments to a function without using the (). The function signature must not change. Using enum type works but not pointer type. I need the template type to be shared_ptr but it only accepts c style pointer type. Even with c style pointer type, the template complains of "cliff_left_2 not useable in a constant expression."

Thanks
Chris

#include <cstdlib>
#include <iostream>
#include <set>
#include <memory>

using namespace std;

enum create_sensor{
		bumper_and_wheeldrop,
		cliff_left,
                cliff_front_left,
                cliff_right,
                cliff_front_right
	};

class Sensor_packet{
public:
        Sensor_packet(std::string i_id,  int i_p_id, int i_size=1): id(i_id), p_id(i_p_id), size(i_size){};
        
        std::string id;
        int size=0; //number of bytes for data
        unsigned char buffer[2];
        unsigned char p_id=0; //packet id
    }; 

template <Sensor_packet * ... type>  //dont know if this is correct, but shared_ptr type does not work either.
int check_sensor(){
   std::set<std::shared_ptr<Sensor_packet> >   result_set {type...};    
   for (auto &x : result_set) {
       std::cout << "value " << x->id << std::endl;
   }
    
}

template <create_sensor ... type> 
int check_sensor2(){
   std::set<create_sensor>   result_set {type...};    
   for (auto &x : result_set) {
       std::cout << "value " << x << std::endl;
   }
    
}


/*
 * 
 */
int main(int argc, char** argv) {
    
    std::shared_ptr<Sensor_packet> bumper_and_wheeldrop2=std::make_shared<Sensor_packet>("BUMP_AND_WHEELDROP",7);
    std::shared_ptr<Sensor_packet> cliff_left2=std::make_shared<Sensor_packet>("CLIFF_LEFT",9);
    
    //check_sensor<cliff_left2,bumper_and_wheeldrop2>();  //does not work
    check_sensor2<cliff_left,bumper_and_wheeldrop>();
    
    return 0;
}

[JLBorges]

> Even with c style pointer type, the template complains of "cliff_left_2 not useable in a constant expression."

There are restrictions on what a template non-type argument can be:

For pointers to objects, the template arguments have to designate the address of an object with static storage duration and a linkage (either internal or external), or a constant expression that evaluates to the appropriate null pointer or std::nullptr_t value. http://en.cppreference.com/w/cpp/language/template_parameters

Something like this, perhaps:

#include <iostream>
#include <string>
#include <set>
#include <memory>

class Sensor_packet {

    public:

        Sensor_packet( std::string i_id /* ... */  ): id(i_id) /* ... */  {};

        std::string id;
        // ...
};

template< typename... T > void check_sensor( std::shared_ptr<T>... shared_pointers ) {

    const std::set< std::shared_ptr<Sensor_packet> > result_set { std::move(shared_pointers)... } ;

    for( const auto& ptr : result_set ) std::cout << ptr->id << '\n' ;
}

int main() {

    check_sensor( std::make_shared<Sensor_packet>("one"), std::make_shared<Sensor_packet>("two"),
                  std::make_shared<Sensor_packet>("three"), std::make_shared<Sensor_packet>("four") ) ;
}

http://coliru.stacked-crooked.com/a/7e97db01cabb5256

[cmisip]

Thanks, however I require the function signature to not change as I am using function pointers in constructors of Behavior objects. I need the function signature to be compatible with :

typedef int(Robot::*m_behavior)(int);

I did some further reading as you pointed out regarding non-type arguments to templates. I came up with this solution. Some of the concepts are still above my grasp. I am confused as to why the non_type arguments need to have linkage but I achieved that by making them global but in an enclosed namespace. Is there anyway to make Sensor_packet variables global in Robot class only? I was wondering if there is a better way. Also do you see any potential problems with the implementation. I try to move away from C type pointers in general and go with shared_ptrs, however, there is no way to use shared_ptr that I could figure out in a template parameter.

Here is my implementation:

#include <cstdlib>
#include <iostream>
#include <set>
#include <memory>

using namespace std;

/*enum create_sensor{
		bumper_and_wheeldrop,
		cliff_left,
                cliff_front_left,
                cliff_right,
                cliff_front_right
	};
*/
class Sensor_packet{
public:
        Sensor_packet(std::string i_id,  int i_p_id, int i_size=1): id(i_id), p_id(i_p_id), size(i_size){};
        
        std::string id;
        int size=0; //number of bytes for data
        unsigned char buffer[2];
        unsigned char p_id=0; //packet id
    }; 


/*template <create_sensor ... type> 
int check_sensor2(){
   std::set<create_sensor>   result_set {type...};    
   for (auto &x : result_set) {
       std::cout << "value " << x << std::endl;
   }
    
}
*/


namespace Sensors{  //necessary to create the required linkage.
      Sensor_packet wheel=Sensor_packet("WHEEL",1);  
      Sensor_packet bumper=Sensor_packet("BUMPER",1); 
}
  
 class Robot{
 public:
     Robot(){ };

     template <Sensor_packet * ... type > //shared_ptr does not work
     int check_sensor(int flag=0){
        std::set<Sensor_packet *> result_set {type...};
        for (auto &x : result_set) {
         std::cout << "value " << x->id << std::endl;
        }
    
     return true;
     }
};



class Behavior{
public:
       typedef int(Robot::*m_behavior)(int);  //the check_sensor function instance needs to conform to this typedef
       m_behavior fptr;
       Robot &r;
       Behavior(m_behavior mbhav, Robot &o) : fptr(mbhav), r(o) {}; //successful constructor call confirms the conformance to the typedef
       void exec(){
           (r.*(this->fptr))(0);
       }
       
};
     

int main(int argc, char** argv) {
    
    Robot create;
    Behavior b(&Robot::check_sensor<&Sensors::wheel,&Sensors::bumper>,create);
    b.exec();
    
    return 0;
}

Thanks,
Chris

[JLBorges]

> I am confused as to why the non_type arguments need to have linkage

A template in instantiated at compile-time; but the instantiated function is called at run-time. The objects involved (the ones pointed to or referred by the template non-type arguments) must be available to the function at run-time.

The instantiated function may be called at any point during the execution of the program; therefore these objects must be available throughout the life of the program; they must have linkage.


> Is there anyway to make Sensor_packet variables global in Robot class only?

The do not have to be global. They could be static members of the Robot class (static members have linkage).


> I try to move away from C type pointers in general and go with shared_ptrs

There is absolutely nothing wrong in using raw non-owning pointers.

In this particular case, the pointers would never be null, and a reference parameter would be appropriate.


With the Sensor_packet variables being static members of Robot, and the template parameters being references:

#include <iostream>
#include <string> // *** required
#include <set>

struct Sensor_packet {

        explicit Sensor_packet( std::string id, int size ) : id(id), size(size) {};

        std::string id;
        int size ;
        // ...
};

 struct Robot {

     template < Sensor_packet&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {

         std::set< Sensor_packet* > result_set { std::addressof(Sensor_packet_objects)... };

         for( Sensor_packet* p : result_set ) std::cout << "value " << p->id << '\n' ;

         return true;
     }

     static Sensor_packet wheel ;
     static Sensor_packet bumper ;
};

Sensor_packet Robot::wheel( "WHEEL", 1 );
Sensor_packet Robot::bumper( "BUMPER", 1 );

struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};

int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< Robot::wheel, Robot::bumper >, robot );
    behaviour.exec();
}

http://coliru.stacked-crooked.com/a/5e77f99611ccf4bb
http://rextester.com/MCF62914

[cmisip]

I am trying to integrate the code into my project but ran into a snag. I am getting this type of error with the set. Thinking that it an issue with std::set, I changed to std::vector but I get the same type of error. I cant seem to be able to pass a vector or a set as an argument to a Biscuit member function. If I modify the code above to include the mechanics of the Biscuit object, it works but of course everything is in one file.

ux/Behaviors.o: In function `int Robot::csensor_l<&sensors::bumper_and_wheeldrop, &sensors::cliff_left>(l_package&)':
/home/cmisip/NB1/CV3-april-obs/Robot.h:205: undefined reference to `Biscuit::biscSensorPacket(std::vector<Sensor_packet*, std::allocator<Sensor_packet*> >& 

Here is the mechanics with Biscuit object.

#include <iostream>
#include <string> // *** required
#include <set>

struct Sensor_packet {

        explicit Sensor_packet( std::string id, int size ) : id(id), size(size) {};

        std::string id;
        int size ;
        // ...
};

class Biscuit {
public:
    int x;
    Biscuit(){};
    void setup_request(std::set<Sensor_packet *> &spacket){
    }
    void battery_check(unsigned char &buffer){
        
    }
};

 struct Robot {

     template < Sensor_packet&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {

         std::set< Sensor_packet* > result_set { std::addressof(Sensor_packet_objects)... };

         for( Sensor_packet* p : result_set ) std::cout << "value " << p->id << '\n' ;
         
         engine.setup_request(result_set);  //does not work in my setup
         unsigned char buf;
         engine.battery_check(buf);          //works
          return true;
     }

     static Sensor_packet wheel ;
     static Sensor_packet bumper ;
     Biscuit engine;
};

Sensor_packet Robot::wheel( "WHEEL", 1 );
Sensor_packet Robot::bumper( "BUMPER", 1 );

struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};

int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< Robot::wheel, Robot::bumper >, robot );
    behaviour.exec();
}

I have the template for check_sensor in Robot.h. I also have a Robot.cpp.

I have a Biscuit.h and Biscuit.cpp.

The Biscuit object member that receives a unsigned char works. Passing a vector or set results in the link error. Any ideas?

Thanks
Chris

[JLBorges]

> If I modify the code above to include the mechanics of the Biscuit object,
> it works but of course everything is in one file.

Verify that the #include guards are correct (not duplicated across files).

Make sure that the non-inline template members are defined in the header file.

[cmisip]

Problem solved. I forgot to add scope resolution to the Biscuit method. Its compiling and linking properly now. I will try to test this on the robot at some point. I will report the results or I might have more questions.

Thanks,
Chris

[cmisip]

Is there anyway to add polymorphism to this solution. I'm thinking of creating a Sensor_packet_bool and Sensor_packet_value class which will inherit from Sensor_packet.
Thanks,
Chris

[JLBorges]

Since in template < Sensor_packet&... Sensor_packet_objects > int Robot::check_sensor( int = 0 ) ; Sensor_packet_objects are actually references to Sensor_packet
and std::set< Sensor_packet* > result_set holds pointers to these objects,
there would be run-time polymorphic behaviour; virtual functions would be dispatched at run time.

[cmisip]

I tried it but it doesn't seem to work.
Chris

#include <iostream>
#include <string> // *** required
#include <set>

struct Sensor_packet {
        explicit Sensor_packet( std::string id, int size ) : id(id), size(size) {};

        std::string id;
        int size ;
        // ...
};

struct Sensor_packet_b : public Sensor_packet {
    Sensor_packet_b(std::string i_id, int i_size) : Sensor_packet(i_id, i_size){}; 
};

class Biscuit {
public:
    int x;
    Biscuit(){};
    void setup_request(std::set<Sensor_packet *> &spacket){
    }
    void battery_check(unsigned char &buffer){
        
    }
};

 struct Robot {
   
     template < Sensor_packet&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {

         std::set< Sensor_packet* > result_set { std::addressof(Sensor_packet_objects)... };
         for( Sensor_packet* p : result_set ) std::cout << "value " << p->id << '\n' ;
         engine.setup_request(result_set);  
        
          return true;
     }

     static Sensor_packet wheel ;
     static Sensor_packet_b bumper ;
     Biscuit engine;
};

Sensor_packet Robot::wheel( "WHEEL", 1 );
Sensor_packet_b Robot::bumper( "BUMPER", 1 );

struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};

int main() {

    Robot robot ;
    //Behavior behaviour( &Robot::check_sensor< Robot::bumper>, robot );  //does not work with derived Sensor_packet_b
    Behavior behaviour( &Robot::check_sensor< Robot::wheel >, robot );  //works
    behaviour.exec();
}

[JLBorges]

> I tried it but it doesn't seem to work.

Yes, my mistake; I didn't know that derived class to base class reference/pointer conversions are not available for template non-type arguments.

Work around: wrap the references and make the template parameter a reference to the wrapped reference.

#include <iostream>
#include <string> // *** required
#include <set>
#include <functional>
#include <typeinfo>

struct Sensor_packet {

        explicit Sensor_packet( std::string id, int size ) : id(id), size(size) {};

        std::string id;
        int size ;
        // ...
        virtual void dummy() const {} // to make it a polymorphic type
};

struct derived_class_Sensor_packet_b : public Sensor_packet { using Sensor_packet::Sensor_packet ; };

class Biscuit {
public:
    int x;
    Biscuit(){};
    void setup_request( std::set<Sensor_packet *>& ){}
    void battery_check(unsigned char& ){}
};

 struct Robot {

     template < std::reference_wrapper<Sensor_packet>&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {

         static const auto addressof = [] ( std::reference_wrapper<Sensor_packet>& r )
         { return std::addressof( r.get() ) ; } ;

         std::set< Sensor_packet* > result_set { addressof(Sensor_packet_objects)... };

         for( Sensor_packet* p : result_set ) if( p != nullptr )
                std::cout << "value " << p->id << " (dynamic type: " << typeid(*p).name() << ")\n" ;

         engine.setup_request(result_set);

          return true;
     }

     static Sensor_packet wheel_ ;
     static derived_class_Sensor_packet_b bumper_ ;
     static std::reference_wrapper<Sensor_packet> wheel ;
     static std::reference_wrapper<Sensor_packet> bumper ;
     Biscuit engine;
};

Sensor_packet Robot::wheel_( "WHEEL", 1 );
derived_class_Sensor_packet_b Robot::bumper_( "BUMPER", 1 );
std::reference_wrapper<Sensor_packet> Robot::wheel( Robot::wheel_ ) ;
std::reference_wrapper<Sensor_packet> Robot::bumper( Robot::bumper_ ) ;

struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};

int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< Robot::wheel, Robot::bumper >, robot );  //works
    behaviour.exec();
}

http://coliru.stacked-crooked.com/a/16a7ef413b5cf811
http://rextester.com/USJF62214

[cmisip]

Thank you very much. I am trying to understand all this a little more deeply. I figured that the answer should eventually lead to a use of pointers but was unable to come up with any solution until your most recent post. I read up on std::reference_wrapper and found it was essentially a pointer to a reference with implicit access to the pointed to reference. I needed to make all these concepts interconnect in my head so I created the following implementation to explore the idea a little bit more. Is there anything inherently dangerous or bad practice in the following code? Are there any memory leaks?

I did realize

virtual void dummy() const {};

was necessary to establish polymorphism. Without it, dynamic_cast wont work but static_cast seems to work in its place. I thought before this that creating a derived class was all that was necessary for polymorphism to be established.

So I think that non type template arguments are actually passed by value including pointers and references. Since pointers can be passed this way, the objects can be recreated in class wrappper from the passed in values as actual objects. Is this how it works?

Thanks,
Chris

#include <iostream>
#include <string> // *** required
#include <vector>
#include <bitset>

  int isNthBitSet (const unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    // c =  00000010 n= 1 ( rightmost bit is n=0, leftmost is n=7), result is 1
    std::bitset<8> x(c);
    std::cout << "This buffer " << x << std::endl;
    return ((c & mask[n]) != 0);
}
  void SetNthBit (unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    // c =  00000010 n= 1 ( rightmost bit is n=0, leftmost is n=7), result is 1
    c=c | mask[n];
    std::bitset<8> x(c);
    std::cout << "New byte " << x << std::endl;
}


enum packet_type {
	bool_type,
	value_type
};

struct Sensor_packet {
        Sensor_packet( std::string id, int size, packet_type ip_type ) : id(id), size(size), p_type(ip_type) {};
        
        std::string id;
        int size ;
        packet_type p_type;
        //virtual void dummy() const {};  //uncomment to allow dynamic_cast to work by establishing polymorphism, otherwise, static_cast works
};

struct Sensor_packet_b : public Sensor_packet {
	
	int bit_offset=0; 
    unsigned char buffer=0x01; //hold the buffer to which a bit setting at bit_offset is checked to arrive at a boolean value
    bool result=false;
    Sensor_packet_b(std::string i_id, int ibit_offset=0) : Sensor_packet(i_id, 1 , packet_type::bool_type ), bit_offset(ibit_offset) { 
		std::bitset<8> x(buffer);
		std::cout << "Initial buffer " << id << " "<< x << std::endl; 
	};
	
    
    
    
};


struct Sensor_packet_v : public Sensor_packet {
    Sensor_packet_v(std::string i_id, int i_size ) : Sensor_packet(i_id, i_size, packet_type::value_type ){}; 
    uint16_t buffer=0x02; //hold the int result for sensor packets that return a value such as distance, angle_turned
};


class Biscuit {
public:
    int x;
    Biscuit(){};
    void setup_request(std::vector<Sensor_packet *> &spacket){  //eventually will build the serial packet request and write to serial device and then read from it
		for (auto &s : spacket) {
			if (s->p_type == packet_type::bool_type ) {
				std::cout << s->id << " is a bool_type " << std::endl;
				Sensor_packet_b * t= static_cast<Sensor_packet_b*>(s);  //dynamic cast wont work, pointer is not polymorphic unless the virtual dummy method is used
				                                                        // but static_cast seems to work
				std::bitset<8> x(t->buffer);
				std::cout << "Original buffer " << x << std::endl; //checking to see if the buffer value was carried over from initialization value
				t->result=isNthBitSet(t->buffer,t->bit_offset);
				
			} 
			else {
				std::cout << s->id << " is a value_type " << std::endl;
				Sensor_packet_v * t= static_cast<Sensor_packet_v*>(s);
				std::bitset<8> x(t->buffer);
				std::cout << "Original buffer " << x << std::endl;
				t->buffer=20;
			}
		}
    }
   
};

template <class T>
class wrapper{
 public:
     T* ptr;
     
     wrapper(T &object){ ptr=&object; };  //create a wrapper object by taking a reference
     
	 ~wrapper(){
		std::cout << "wrapper destructor" << std::endl;
	 }
     
};


 struct Robot {
   
     template <  wrapper<Sensor_packet>&... Sensor_packet_objects >
     //template <  wrapper<Sensor_packet>*... Sensor_packet_objects > //equivalent code when passing a pointer
     int check_sensor( int = 0 ) {
         std::vector< wrapper<Sensor_packet>*> l_list { std::addressof(Sensor_packet_objects)... } ;
         //std::vector< wrapper<Sensor_packet>*> l_list { Sensor_packet_objects... } ; //equivalent code when passing a pointer
         std::vector< Sensor_packet* > s_list;
         
         //Convert to vector of Sensor_packet*
         for (auto &p :l_list) {
			 s_list.push_back(p->ptr);
		 }
         
         std::cout << "Size of vector of wrapper" << l_list.size() << std::endl;
         for( auto &p : l_list ) std::cout << "id " << p->ptr->id << '\n' ;
         
         std::cout << "Converted to vector of Sensor_packet" << std::endl;
         for( auto &p : s_list ) std::cout << "id " << p->id << '\n' ;
         
         engine.setup_request(s_list);  //send the vector as a reference
         
         std::cout << "Checking results" << std::endl;
         for( auto &p : s_list ) { 
			 if (p->p_type == bool_type) {
			    std::cout << p->id << " bool " << static_cast<Sensor_packet_b*>(p)->result << '\n' ;
			 } else
			    std::cout << p->id << " value " << static_cast<Sensor_packet_v*>(p)->buffer << '\n' ;
         }
          return true;
     }
     

     Biscuit engine;
};

Sensor_packet_b wheel( "WHEEL", 0  ); //id, bit_offset 
Sensor_packet_v bumper( "BUMPER", 1 ); //id, buffer

// create a wrapper object using a Sensor_packet object
wrapper<Sensor_packet> wbumper(bumper); //value_type
wrapper<Sensor_packet> wwheel(wheel); //value_type


struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};


int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< wwheel, wbumper >, robot );
    //Behavior behaviour( &Robot::check_sensor< &wwheel, &wbumper >, robot );  //equivalent code when passing pointers
    behaviour.exec();
}

[JLBorges]

> I figured that the answer should eventually lead to a use of pointers

The answer should be based on a statically determined type (without any implicit type conversion).
Any wrapper which collapses the different types into a single static type would do; for instance, a pointer or reference to C++17 variant http://en.cppreference.com/w/cpp/utility/variant should work quite nicely.


> non type template arguments are actually passed by value

The template non-type arguments are treated akin to literals. They are not 'passed' (at run-time); template instantiation takes place at compile-time.

A template-argument for a non-type template-parameter shall be a converted constant expression of the type of the template-parameter

and

A converted constant expression of type T is an expression, implicitly converted to type T, where the converted expression is a constant expression and the implicit conversion sequence contains only
- user-defined conversions,
- lvalue-to-rvalue conversions,
- array-to-pointer conversions,
- function-to-pointer conversions,
- qualification conversions,
- integral promotions,
- integral conversions other than narrowing conversions,
- null pointer conversions from std::nullptr_t,
- null member pointer conversions from std::nullptr_t, and
- function pointer conversions,
and where the reference binding (if any) binds directly.

[Note: such expressions may be used in new expressions, as case expressions, as enumerator initializers if the underlying type is fixed, as array bounds, and as non-type template arguments - end note ]

> Is there anything inherently dangerous or bad practice in the following code? Are there any memory leaks?

There is no memory leak since objects with dynamic storage duration are not involved.
Can't say anything more without knowing the context: for instance, why is it required that the type of the pointer to function must be typedef int(Robot::*m_behavior)(int);

To make the code more robust, consider
a. validating the bit positions:

#include <limits>
#include <type_traits>
#include <stdexcept>

template < typename T > bool valid_bit( unsigned int n ) {

    static_assert( std::is_integral<T>::value && std::is_unsigned<T>::value,
                   "expected an unsigned integral type" ) ;
    static constexpr std::size_t NBITS = std::numeric_limits<T>::digits ;
    return n < NBITS ;
}

bool isNthBitSet( unsigned char c, unsigned int n ) { // favour value semantics

    return valid_bit<unsigned char>(n) ? c & ( 1U << n ) : false ;
}

unsigned char SetNthBit( unsigned char c, unsigned int n ) { // favour value semantics

    if( valid_bit<unsigned char>(n) ) return c |= ( 1U << n ) ;
    else throw std::invalid_argument("invalid bit position" ) ;
}

b. use std::addressof to get the address in the constructor of wrapper (T may be a type with an overloaded unary & operator).

c. in Biscuit::setup_request, verify that the pointer is not null before trying to access the object.

[cmisip]

Thanks for the pointers ( no pun intended ). I will look at applying all this to my project.

Regarding, the typedef, It was a design decision. I started out creating Robot class and filled it with a ton of functions that were reasonably similar in signature. I found out that I was implementing a lot of the same things in the different functions with no particular order or system so I decided that maybe there was a way that I could modularize things a bit and build what I call macrobehaviors with component microbehaviors. So with polymorphic Behaviors and MicroBehaviors and MacroBehaviors objects, I put together a system that chained (programmed) a sequence of micro's into a macro. Each micro is assigned a function pointer from the functions in Robot class to execute and therefore, the function pointers must be a specific signature. So the old system and the new system are merged.

I have learned a lot from this one particular post. I will report in the future if I have succeeded in integrating the above code into my project or I might have more questions.

The above technique is a very good way of 'passing' variables into functions ('instantiated' at compile time) without having to use the (), albeit in a round about way using literals. The effect is the same.

With regards to shared_ptr's, they are wrappers like std::reference_wrapper or the wrapper class above. I thought about this and came up with the following implementation using shared_ptr's just for completeness.

Thanks
Chris

#include <iostream>
#include <string> // *** required
#include <vector>
#include <bitset>
#include <memory>

  int isNthBitSet (const unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    // c =  00000010 n= 1 ( rightmost bit is n=0, leftmost is n=7), result is 1
    std::bitset<8> x(c);
    std::cout << "This buffer " << x << std::endl;
    return ((c & mask[n]) != 0);
}
  void SetNthBit (unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    // c =  00000010 n= 1 ( rightmost bit is n=0, leftmost is n=7), result is 1
    c=c | mask[n];
    std::bitset<8> x(c);
    std::cout << "New byte " << x << std::endl;
}


enum packet_type {
	bool_type,
	value_type
};


struct Sensor_packet {
        Sensor_packet( std::string id, int size, packet_type ip_type ) : id(id), size(size), p_type(ip_type) {};
        
        std::string id;
        int size ;
        packet_type p_type;
        //virtual void dummy() const {};  //uncomment to allow dynamic_cast to work by establishing polymorphism, otherwise, static_cast works
};

struct Sensor_packet_b : public Sensor_packet {
	
	int bit_offset=0; 
    unsigned char buffer=0x01; //hold the buffer to which a bit setting at bit_offset is checked to arrive at a boolean value
    bool result=false;
    Sensor_packet_b(std::string i_id, int ibit_offset=0) : Sensor_packet(i_id, 1 , packet_type::bool_type ), bit_offset(ibit_offset) { 
		std::bitset<8> x(buffer);
		std::cout << "Initial buffer " << id << " "<< x << std::endl; 
	};
	
	
    
    
    
};


struct Sensor_packet_v : public Sensor_packet {
    Sensor_packet_v(std::string i_id, int i_size ) : Sensor_packet(i_id, i_size, packet_type::value_type ){}; 
    uint16_t buffer=0x02; //hold the int result for sensor packets that return a value such as distance, angle_turned
    
   
};



class Biscuit {
public:
    int x;
    Biscuit(){};
    void setup_request(std::vector<std::shared_ptr<Sensor_packet> >  &spacket){  //eventually will build the serial packet request and write to serial device and then read from it
		for (auto &s : spacket) {
			if (s->p_type == packet_type::bool_type ) {
				std::cout << s->id << " is a bool_type " << std::endl;
				std::shared_ptr<Sensor_packet_b> t= std::static_pointer_cast<Sensor_packet_b>(s);  //dynamic cast wont work, pointer is not polymorphic unless the virtual dummy method is used
				                                                        // but static_cast seems to work
				std::bitset<8> x(t->buffer);
				std::cout << "Original buffer " << x << std::endl; //checking to see if the buffer value was carried over from initialization value
				t->result=isNthBitSet(t->buffer,t->bit_offset);
				
			} 
			else {
				std::cout << s->id << " is a value_type " << std::endl;
				std::shared_ptr<Sensor_packet_v> t= std::static_pointer_cast<Sensor_packet_v>(s);
				std::bitset<8> x(t->buffer);
				std::cout << "Original buffer " << x << std::endl;
				t->buffer=20;
			}
		}
    }
   
};

template <class T>
class wrapper{
 public:
     T* ptr;
     
     wrapper(T &object){ ptr=&object; };  //create a wrapper object by taking a reference
     
	 ~wrapper(){
		std::cout << "wrapper destructor" << std::endl;
	 }
     
};


 struct Robot {
     template <  std::shared_ptr<Sensor_packet>&... Sensor_packet_objects > 
     int check_sensor( int = 0 ) {
      /*   std::vector< std::shared_ptr<Sensor_packet>*> l_list { std::addressof(Sensor_packet_objects)... } ; //l_list is a vector of shared_ptr's
         std::vector< std::shared_ptr<Sensor_packet > > s_list;  
         
         std::cout << "Size of vector of shared_ptr" << l_list.size() << std::endl;
         for( auto &p : l_list ) std::cout << "id " << (*p)->id << '\n' ;
         
         //Convert to vector of std::shared_ptr<Sensor_packet>
         for (auto &p :l_list) {
			 s_list.push_back((*p)); //once dereference to get the shared_ptr
		 }

     */
         std::vector< std::shared_ptr<Sensor_packet > > s_list { *(std::addressof(Sensor_packet_objects))... } ;

         //Reconstituted after passing through the teleporter. 
         
         std::cout << "Converted to vector of std::shared_ptr<Sensor_packet>" << std::endl;
         for( auto &p : s_list ) std::cout << "id " << p->id << '\n' ;
         
         engine.setup_request(s_list);  //send the vector as a reference
         std::cout << "Checking results" << std::endl;
         for( auto &p : s_list ) { 
			 if (p->p_type == bool_type) {
			    std::cout << p->id << " bool " << std::static_pointer_cast<Sensor_packet_b>(p)->result << '\n' ;
			 } else
			    std::cout << p->id << " value " << std::static_pointer_cast<Sensor_packet_v>(p)->buffer << '\n' ;
         
         }
         
         
         
          return true;
     }
     

     Biscuit engine;
};


std::shared_ptr<Sensor_packet> wbumper=std::make_shared<Sensor_packet_v>( "BUMPER", 0  );
std::shared_ptr<Sensor_packet> wwheel=std::make_shared<Sensor_packet_b>( "WHEEL", 0  );

struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};


int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< wwheel, wbumper >, robot );  //Chop it into molecular bits, Energize !
    behaviour.exec();
}

[JLBorges]

> Each micro is assigned a function pointer from the functions in Robot class
> to execute and therefore, the function pointers must be a specific signature.

You may want to consider wrapping the callable objects in the polymorphic call wrapper std::function<>
http://en.cppreference.com/w/cpp/utility/functional/function

Things become very flexible when std::function<> is used in conjunction with std::bind()
http://en.cppreference.com/w/cpp/utility/functional/bind

[cmisip]

I do a lot of ruminating when I am driving. It occurred to me that since wwheel and wbumper have global status (linkage) and even if they are static variables of Robot class, they are naturally available (in scope) to check_sensor without having to go through the template route. So that

                 s_list.push_back(::wwheel);
		 s_list.push_back(::wbumper);

in check_sensor actually works to populate s_list. The linkage as a requirement to using pointers in template <> pretty much makes this way of using them moot but it does express the programmer's intention. It was a good theoretical exercise. I wonder if there is any way at all that this could work without the linkage requirement.

Thank You for the suggestion on using std::function and std::bind. Upon reading, I think, the way it works is that it abstracts the heterogeneity of the callable objects (in my case functions in Robot class) such that I have a consistent function signature to assign to the function pointer in Behavior class. I think that is how it is supposed to work. This opens up possibilities as I will no longer be constrained by a particular function signature.

Thanks again
Chris

[cmisip]

@JLBorges

I am trying to optimize the above solution and I was thinking that shared_ptr's are probably a bad idea performance wise as the objects will be allocated on the heap and the (static or global) Sensor_packet objects never really go out of scope. This kinda makes the point of shared_ptr use (reference counting and automatic object destruction) moot as it is only used as a wrapper just with the overhead. I am going to run check_sensor in a parallel thread and it will serve as my replacement for sensor streaming which I couldn't get to work on the Irobot. I put in a virtual getter function for the Sensor_packet*->buffer which eliminate the need for a pointer_cast. (Dont know if using virtual functions will be better than using static_cast but I assume the former is better). I decide to use the wrapper class in an earlier version of the code but I modified it in the fashion of shared_ptr invocation so I do the wrapper object and Sensor_packet object creation in one line. Likewise, the creation of a vector of s_list in check_sensor is now a single line with no need for creating a vector of l_list and creating s_list from l_list (as I did previously).

packet_request() and packet_receive() need to happen one after the other because there is a delay between the serial request and when the serial device actually sends the reply and there is a mutex controlling access to the serial device. I dont want to hog the serial device by introducing a usleep between packet request and packet receive if they are imlemented in one function because other functions may need to send commands ( for example if I need to send a drive command from somewhere else).

So I seem to have a race condition. Some functions running in parallel with check_sensor will need to access the values of Sensor_packet*->buffer but is there a possibility that the buffer is accessed midupdate? Or are these updates to unsigned char buffer or signed in buffer atomic. I cant use std::atomic type as class members of Sensor_packet objects because they are noncopyable.

I am running this code on a raspberry pi and I am wondering if operations on uchar and ints on this platform are intrinsically atomic.

Also please let me know if you have other ideas on optimizing the check_sensor function.

This is the working code so far.

Thanks
Chris

 
#include <iostream>
#include <string> // *** required
#include <vector>
#include <bitset>
#include <mutex>
#include <atomic>

  int isNthBitSet (const unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    // c =  00000010 n= 1 ( rightmost bit is n=0, leftmost is n=7), result is 1
    std::bitset<8> x(c);
    std::cout << "This buffer " << x << std::endl;
    return ((c & mask[n]) != 0);
}
  void SetNthBit (unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    // c =  00000010 n= 1 ( rightmost bit is n=0, leftmost is n=7), result is 1
    c=c | mask[n];
    std::bitset<8> x(c);
    std::cout << "New byte " << x << std::endl;
}


enum packet_type {
	bool_type,
	value_type
};

struct Sensor_packet {
        Sensor_packet( std::string id, int size, packet_type ip_type ) : id(id), size(size), p_type(ip_type) {};
        Sensor_packet(){};
        std::string id;
        int size ;
        packet_type p_type;
        virtual signed int get() = 0;
        virtual void set(int x) = 0;  //set is only for diagnostic as buffer will be directly written to during serial device read
};

struct Sensor_packet_b : public Sensor_packet {
    int bit_offset=0; 
    unsigned char buffer=0x01; //hold the buffer to which a bit setting at bit_offset is checked to arrive at a boolean value, probably needs to be atomic type
    bool result=false;
    Sensor_packet_b(std::string i_id, int ibit_offset=0) : Sensor_packet(i_id, 1 , packet_type::bool_type ), bit_offset(ibit_offset) { 
		std::bitset<8> x(buffer);
		std::cout << "Initial buffer " << id << " "<< x << std::endl; 
	};
    Sensor_packet_b(){};
    signed int get() {
        return (signed int) isNthBitSet(buffer,bit_offset);
    }
    
    void set(int x){
        buffer=x;
    }
    
};


struct Sensor_packet_v : public Sensor_packet {
    Sensor_packet_v(std::string i_id, int i_size ) : Sensor_packet(i_id, i_size, packet_type::value_type ){}; 
    signed int buffer=0x02; //hold the int result for sensor packets that return a value such as distance, probably needs to be atomic type
    Sensor_packet_v(){};
    signed int get(){
        return buffer;
    }
    
    void set(int x){
        buffer=x;
    }
};


class Biscuit {
public:
    int x;
    std::recursive_mutex s_mutex;
    Biscuit(){};
    
    Biscuit& operator=(const Biscuit&) = delete; //no copies because we have a noncopyable mutex

    Biscuit(const Biscuit& rhs) = delete; //no assignment because we have a noncopyable mutex
    
    void  packet_request(std::vector<Sensor_packet *> &spacket){  
                std::lock_guard<std::recursive_mutex> lock(s_mutex);
                //serial write here

                //bad idea to do a usleep here ( in case a unified packet write and packet read function is to be written )  while waiting for serial stream to read as we will be locking the serial device during the wait, preventing other functions from sending serial write commands
                
                //following just checks if we really got spacketset reference intact
		for (auto &s : spacket) {
				std::bitset<8> x(s->get());
				std::cout << "Original buffer " << x << " with int value of " << s->get() <<  std::endl; //checking to see if the buffer value was carried over from initialization value
		}
    }
    
    void packet_receive(std::vector<Sensor_packet *> &spacket){
                std::lock_guard<std::recursive_mutex> lock(s_mutex); 
               //serial read here which modifies the buffer members of spacket elements
                
                //try to modify spacket and see if we get the changes back in check_sensor
                for (auto &s : spacket) {
                    s->set(0);
		}
    }
};


template <class T>
class wrapper {
    public:
    T *ptr;

    template <class U>
    wrapper(U& obj) {
        ptr=&obj;
    }
};


template <class U>
U& make_object(std::string id, int x){
    static U object;
    object=U(id,x);
    return object;
}


//one liner wrapper object and Sensor_packet object creation
wrapper<Sensor_packet> wwheel=make_object<Sensor_packet_b>("WHEEL", 0);
wrapper<Sensor_packet> wbumper=make_object<Sensor_packet_v>("BUMPER", 1);


 struct Robot {
   
     template <  wrapper<Sensor_packet>&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {
         std::vector<Sensor_packet* >  s_list { (std::addressof(Sensor_packet_objects)->ptr)... } ; 
         
         engine.packet_request(s_list);  //send the vector as a reference to aid in creation of packet request
         
         //The Biscuit mutex is out of scope here so others can access the serial device ( such as if I want to send a drive command from running behavior in main()).
         //Its ok to introduce a usleep here because the mutex is not locked.
         
         engine.packet_receive(s_list);  //receive the vector with updated values
         
         //the following is just a test to see if we got s_list reference back with modifications
         std::cout << "Checking results" << std::endl;
         for( auto &p : s_list ) { 
             std::cout << p->id << " shows " << p->get() << std::endl;
         }
       
          return true;
     }
     

     Biscuit engine;
};




struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};


int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< wwheel, wbumper >, robot );
    behaviour.exec();
}

[JLBorges]

> I am trying to optimize the above solution and I was thinking that
> shared_ptr's are probably a bad idea performance wise

Yes; unnecessary shared ownership is also a bad idea from a design perspective.
Use raw (non-owning) pointers for this; non-owning raw pointers are canonical in C++.

If performance is a concern, an object-oriented Sensor_packet is contra-indicated. From the snippets that are posted, a union-like class appears to be more appropriate. Something like:

struct Sensor_packet
{
    enum packet_type { bool_type, int_type };

    std::string id ;

    packet_type type = bool_type ;
    union
    {
       bool bit = true ; // type == bool_type
       int int_value ; // type == int_type
    };

    Sensor_packet( std::string id ) : id( std::move(id) ) {}

    Sensor_packet( std::string id, std::pair< unsigned char, std::size_t> bit_and_offset )
        : id( std::move(id) ), type(bool_type),
          bit( nth_bit( bit_and_offset.first, bit_and_offset.second ) ) {}

    Sensor_packet( std::string id, int value )
        : id( std::move(id) ), type(int_type), int_value(value) {}

    int get() const { return bool_type ? bit : int_value ; }

    void set( std::pair< unsigned char,std::size_t> bit_and_offset ) {

        type = bool_type ;
        bit = nth_bit( bit_and_offset.first, bit_and_offset.second ) ;
    }

    void set( int value ) {

        type = int_type ;
        int_value = value ;
    }

    static bool nth_bit( unsigned char bits, std::size_t bit_offset )
    { return bit_offset < 8U ? bits & ( 1U << bit_offset ) : false ; }

    friend std::ostream& operator<< ( std::ostream& stm, const Sensor_packet& pkt )
    {
        stm << "{ " << std::quoted(pkt.id) << ", " << std::boolalpha ;
        if( pkt.type == pkt.bool_type ) return stm << "bool:" << pkt.bit << " )" ;
        else return stm << "value:" << pkt.int_value << " }" ;
    }
};

And then, the hierarchy-collapsing surrogate wrapper won't be required any more. Since the size of the sequence is a (compile-time) integral constant expression, we could use std::array<> instead of std::vector.

With Sensor_packet as a union-like class, sans any wrapper class, and with std::array<> substituting std::vector:

#include <iostream>
#include <string> // *** required
#include <array>
#include <bitset>
#include <mutex>
#include <atomic>
#include <iomanip>
#include <utility>

struct Sensor_packet
{
    enum packet_type { bool_type, int_type };

    std::string id ;

    packet_type type = bool_type ;
    union
    {
       bool bit = true ; // type == bool_type
       int int_value ; // type == int_type
    };

    Sensor_packet( std::string id ) : id( std::move(id) ) {}

    Sensor_packet( std::string id, std::pair< unsigned char,std::size_t> bit_and_offset )
        : id( std::move(id) ), type(bool_type),
          bit( nth_bit( bit_and_offset.first, bit_and_offset.second ) ) {}

    Sensor_packet( std::string id, int value )
        : id( std::move(id) ), type(int_type), int_value(value) {}

    int get() const { return bool_type ? bit : int_value ; }

    void set( std::pair< unsigned char,std::size_t> bit_and_offset ) {

        type = bool_type ;
        bit = nth_bit( bit_and_offset.first, bit_and_offset.second ) ;
    }

    void set( int value ) {

        type = int_type ;
        int_value = value ;
    }

    static bool nth_bit( unsigned char bits, std::size_t bit_offset )
    { return bit_offset < 8U ? bits & ( 1U << bit_offset ) : false ; }

    friend std::ostream& operator<< ( std::ostream& stm, const Sensor_packet& pkt )
    {
        stm << "{ " << std::quoted(pkt.id) << ", " << std::boolalpha ;
        if( pkt.type == pkt.bool_type ) return stm << "bool:" << pkt.bit << " )" ;
        else return stm << "value:" << pkt.int_value << " }" ;
    }
};

class Biscuit {

    public:
        int x = 0 ;

        // implicitly declared default constructor
        // implicitly deleted copy constructor, copy assignment operator

        template< std::size_t N >
        void  packet_send( const std::array< Sensor_packet*, N >& spacket ) { // consumer

            // serial write here
            std::cout << "packet_send " ;
            for( auto p : spacket ) if(p) std::cout << *p << ' ' ;
            std::cout << '\n' ;
        }

        template< std::size_t N >
        void packet_receive( const std::array< Sensor_packet*, N >& spacket ) { // producer

            // serial read here which modifies the buffer members of spacket elements
            std::cout << "packet_receive " ;
            for( auto p : spacket ) if(p) {

                // modify the values for testing purposes
                if( p->type == p->bool_type ) p->set( { p->get() ? 0:1, 1 } ) ; // flip bool
                else p->set( p->get() + 186 ) ; // add 186 to int
                std::cout << *p << ' ' ;
            }
            std::cout << '\n' ;
        }
};

Sensor_packet wheel( "WHEEL", { 1, 0 } ); // unsigned char and offset: Sensor_packet::bool_type
Sensor_packet bumper( "BUMPER", 78 ); // Sensor_packet::value_type

 struct Robot {

     template < Sensor_packet&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {

         std::array< Sensor_packet*, sizeof...( Sensor_packet_objects) >  s_list
                                  { { std::addressof(Sensor_packet_objects)... } } ;

         engine.packet_send(s_list);  //send the array as a reference to aid in creation of packet request

         engine.packet_receive(s_list);  //receive the array with updated values

         //the following is just a test to see if we got s_list reference back with modifications
         std::cout << "Checking results " ;
         for( auto p : s_list ) std::cout << *p << ' ' ;
         std::cout << '\n' ;

         return true;
     }

     Biscuit engine;
};

struct Behavior {

    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};

int main() {

    Robot robot ;
    Behavior behaviour( &Robot::check_sensor< wheel, bumper >, robot );
    behaviour.exec();
}

http://coliru.stacked-crooked.com/a/77c233adbce989c2


I haven't really understood the concurrency scenario in the code. From what I've gathered:

If everywhere in the program, the sequence is packet_request() with a newly created sequence of packets, wait for its completion, then packet_receive(), wait for its completion, no additional synchronisation is required.

If it is a producer-consumer scenario, then std::condition_variable used in conjunction with std::unique_lock<std::mutex> would be the appropriate primitive provided by the library.
http://en.cppreference.com/w/cpp/thread/condition_variable

[cmisip]

Thanks for the reply. It will take me a while to digest this. I have not come across unions or union like classes yet.

The concurrency issue is demonstrated with my earlier post in the beginner forum:

http://www.cplusplus.com/forum/beginner/196573/

Basically, main will loop infinitely "running the current state" which is a behavior object enclosing a function pointer that is pointing to a function in Robot class. In that example, the behavior object is pointed to move_and_scan(int x = distance to travel from which time to travel is calculated and a serial drive stop command is issued when the timer expires). Parallel to this in separate threads run the "listeners" which would be polling sensors or user input to try to influence move_and_scan by stopping the robot motion in case for example keyboard_interrupt(int &x) detected a specific key press. ( I was able to move the robot around using the arrow keys this way ). My idea is to use check_sensors above as a listener that will run in parallel to the running behavior, updating the static sensor_packet objects with data obtained from the serial port. Other listeners will run in parallel again to evaluate the data in specific sensor_packet objects. For example, I could have a bumper_listener that just querries the sensor_packet wbumper and issues a stop directive to move_and_scan() if it evaluates to true. The concurrency problem is that check_sensor may be trying to write to sensor_packet wbumper while bumper_listener is reading the value.

With regards to the use of the mutex in Biscuit, I thought it was necessary because there was a possiblity that move_and_scan will send a serial motor drive or stop command while check_sensor is sending a packet request to the serial port. I split the check_sensor into packet_send and packet_receive because of the delayed response which I did not want to code a usleep in Biscuit as it delays move_and_scan from issuing a stop when its movement timer is expired.

The use of threads is really bad design considering performance, but my main objective was to try to construct macrobehaviors by chaining behaviors and appending listeners to them in a modular and scriptable way. Eventually, the macrobehavior becomes a "state" that is run by main and when it is finnished and no other states are queued in the behavior stack, it will repeatedly run idle_and_scan(int x). Idle_and_scan() will not be completely idle as it will scan the environment and queue appropriate macrobehavior to the Behavior stack if it finds something interesting.

The design decision also had another impact that I haven't figured out yet the solution for. The availability of the serial port to move_and_scan() influences when the actual drive stop command is issued from a timer expiration. If the serial port is not immediately available, then the robot's transit time is increased causing inaccurate dead reckoning calculation. Somehow, the main() thread running move_and_scan needs to have priority mutex access to the serial port. Barring that, the accurate stop time should be obtained at the moment that one of the listeners detected a condition to stop the driving which means the listeners will need to issue the stop command directly and record the time to a variable, instead of passing on the directive to stop to move_and_scan.

Thanks,
Chris

[cmisip]

I wrote this attempt to add std::atomic types to the Sensor_packet objects. I dont quite know how to do it for a union class so I went with the original polymorphic sensor_packet objects. I did implement the optimizations that you put in such as using std::move for id and converting from vector to array. Those changes alone brought the execution time down to 1 clock tick on my HP 8300 (from about 10 clock ticks with iostream stuff commented out). If I left the iostream stuff uncommented, the clock_ticks were as high as 40. I didn't realize the iostream stuff was that slow. I have it all over the place for debugging purposes in my research project.

I think the union class way is still faster than polymorphic way since there is no vtable. I think static_cast is faster than dynamic polymorphism, but the convenience of having a getter function is a good trade_off. Implementation below takes one clock_tick on my HP 8300.

I have to research still how to implement atomic union types.

In my research, I gathered that the reason for no copy constructor or assignment operators with std::atomic types is because some may require a mutex (not lock free) for atomicity. So the general case must not have copy constructors or assignment operators.

Having established that uchar and signed int are lock free, I put in the implementation for their copy constructors and assignment operators.

Do you think that the atomicity of write and read operations on the sensor_packet buffer is intact with the implementation in the code below? I am not quite sure of the memory_order options at this point.

I will try to implement a version with union class that has copyable atomic types.

Thanks
Chris

#include <iostream>
#include <string> // *** required
#include <vector>
#include <bitset>
#include <mutex>
#include <atomic>
#include <time.h> 
#include <stdio.h>

  int isNthBitSet (const unsigned char &c, int n) {
    static unsigned char mask[] = {1, 2, 4, 8, 16, 32, 64, 128};
    std::bitset<8> x(c);
    return ((c & mask[n]) != 0);
}
 

enum packet_type {
	bool_type,
	value_type
};

struct Sensor_packet {
        Sensor_packet( std::string id, int size, packet_type ip_type ) : id(id), size(size), p_type(ip_type) {};
        Sensor_packet(){};
        std::string id;
        int size ;
        packet_type p_type;
        virtual signed int get() = 0;
        virtual void set(int x) = 0;  //set is only for diagnostic as buffer will be directly written to during serial device read
};


struct Sensor_packet_b : public Sensor_packet {
    int bit_offset=0; 
    std::atomic<unsigned char> buffer{0x01}; 
    bool result=false;
    Sensor_packet_b(std::string i_id, int ibit_offset=0) : Sensor_packet(std::move(i_id), 1 , packet_type::bool_type ), bit_offset(ibit_offset) { 
		std::bitset<8> x(buffer);
		std::cout << "Initial buffer for sensor_packet_b " << id << " "<< x << std::endl; 
	};
    Sensor_packet_b(){};
    
    Sensor_packet_b(const Sensor_packet_b& pOther) : Sensor_packet(pOther.id, 1 , packet_type::bool_type ), bit_offset(pOther.bit_offset), result(pOther.result)
    {
        buffer.store(pOther.buffer.load(std::memory_order::memory_order_seq_cst), std::memory_order::memory_order_seq_cst);
    }
    
    Sensor_packet_b& operator=(const Sensor_packet_b& pOther)
    {
        if (this != &pOther) {
          buffer.store(pOther.buffer.load(std::memory_order::memory_order_seq_cst), std::memory_order::memory_order_seq_cst);
          id=pOther.id;
          size=pOther.size;
          p_type=pOther.p_type;
        }
        return *this;
    }
    
    signed int get() {
        return (signed int) isNthBitSet(buffer,bit_offset);
    }
    
    void set(int x){
        buffer=x;
    }
    
};

struct Sensor_packet_v : public Sensor_packet {
    Sensor_packet_v(std::string i_id, int i_size ) : Sensor_packet(std::move(i_id), i_size, packet_type::value_type ){
        std::bitset<8> x(buffer);
        std::cout << "Initial buffer for sensor_packet_v " << id << " "<< x << std::endl; 
    }; 
    std::atomic<signed int> buffer{0x02};
    Sensor_packet_v(){};
    
    Sensor_packet_v(const Sensor_packet_v& pOther) : Sensor_packet(pOther.id, pOther.size , packet_type::value_type )
    {
        buffer.store(pOther.buffer.load(std::memory_order::memory_order_seq_cst), std::memory_order::memory_order_seq_cst);
    }
    
    Sensor_packet_v& operator=(const Sensor_packet_v& pOther)
    {
        if (this != &pOther) {
          buffer.store(pOther.buffer.load(std::memory_order::memory_order_seq_cst), std::memory_order::memory_order_seq_cst);
          id=pOther.id;
          size=pOther.size;
          p_type=pOther.p_type;
        }
        return *this;
    }
    
    signed int get(){
        return buffer;
    }
    
    void set(int x){
        buffer=x;
    }
};



class Biscuit {
public:
    int x;
    std::recursive_mutex s_mutex;
    Biscuit(){};
    
    Biscuit& operator=(const Biscuit&) = delete; //no copies because we have a noncopyable mutex

    Biscuit(const Biscuit& rhs) = delete; //no assignment because we have a noncopyable mutex
    
    template< std::size_t N >
    void  packet_request(const std::array< Sensor_packet*, N > &spacket){  
                std::lock_guard<std::recursive_mutex> lock(s_mutex);
    }
    
    template< std::size_t N >
    void packet_receive(const std::array< Sensor_packet*, N > &spacket){
                std::lock_guard<std::recursive_mutex> lock(s_mutex); 
               //serial read here which modifies the buffer members of spacket elements
                
                //try to modify spacket and see if we get the changes back in check_sensor
                for (auto &s : spacket) {
                    std::cout << "Changing " << s->id << " to 0 "<< std::endl; 
                    s->set(0);
		}
                
    }
};

template <class T>
class wrapper {
    public:
    T *ptr;

    template <class U>
    wrapper(U& obj) {
        ptr=&obj;
    }
};


template <class U>
U& make_object(std::string id, int x){
    static U object;
    object=U(id,x);
    return object;
}

wrapper<Sensor_packet> wwheel=make_object<Sensor_packet_b>("WHEEL", 0);
wrapper<Sensor_packet> wbumper=make_object<Sensor_packet_v>("BUMPER", 1);

 struct Robot {
     Biscuit engine;
     template <  wrapper<Sensor_packet>&... Sensor_packet_objects >
     int check_sensor( int = 0 ) {
         
         std::array< Sensor_packet*, sizeof...( Sensor_packet_objects) >  s_list
                                { (std::addressof(Sensor_packet_objects)->ptr)... } ;
         
         engine.packet_request(s_list);  //send the vector as a reference to aid in creation of packet request
         
         //The Biscuit mutex is out of scope here so others can access the serial device ( such as if I want to send a drive command from running behavior in main())
         
         engine.packet_receive(s_list);  //receive the vector with updated values
         
         //the following is just a test to see if we got s_list reference back with modifications
         std::cout << "Checking results" << std::endl;
         for( auto &p : s_list ) { 
             std::cout << p->id << " shows " << p->get() << std::endl;
         }
          return true;
     }
};

struct Behavior {
    typedef int (Robot::*m_behavior) (int) ;
    m_behavior behaviour ;
    Robot& robot ;

    Behavior( m_behavior mbhav, Robot& robot ) : behaviour(mbhav), robot(robot) {}

    void exec() { (robot.*behaviour)(0) ; }
};

int main() {

    Robot robot ;
     clock_t t;
     t=clock();
    Behavior behaviour( &Robot::check_sensor< wwheel, wbumper >, robot );
    
    behaviour.exec();
    t = clock() - t;
    printf ("It took me %ld clicks (%f seconds).\n",t,((float)t)/CLOCKS_PER_SEC);
    
    std::cout << "signed int is lock free : " << std::atomic<signed int>{}.is_lock_free() << std::endl;
    std::cout << "unsigned char is lock free: " << std::atomic<unsigned char>{}.is_lock_free() << std::endl;
    
}