On 2 Oct 2003 13:47:49 -0700,
mpowell_jr@hotmail.com (MPowell) wrote:
[color=blue][color=green]
>> Well, I can fix the errors in the code, but you'll have to explain
>> what the code is meant to do. What do you want to do with the messages
>> that you receive? Do you need to perform some action depending on the
>> type of message you receive?[/color]
>
>Consider[/color]
The below doesn't answer the question, a couple of paragraphs of text
might.
[color=blue]
>
>#define ID_ERROR_MASK 0x08 // #defined at top of the translation unit
>#define ID_MASK 0x07 // #defined at top of the translation unit
>
>
>int _msg2_header_error_count = 0;
>int _msg1_header_error_count = 0;
>
>int _msg2_message_index = 0;
>int _msg1_message_index = 0;
>// lots more
>
>struct _MSG2_STATUS // here for demo and lots more of these
>{
> // for simplicity strip out 'stuff'.
> unsigned int idx : 1;
> unsigned int ldx : 32;
>} MSG2_STATUS;
>
>MSG2_STATUS _msg2_message[ 2 ]; // for double buffering[/color]
I now finally understand what you mean by double buffering - you want
to keep the last two messages of each type.
[color=blue]
>// lots more
>
>The Create function will look at the msg_id (which it does) then check
>for error.
>
> if (msg_id & ID_ERROR_MASK) == 0 )
> {
> // if no error look at msg_type
> MSG_HEADER msg_type =
> (((( msg_id >> 4 ) & ID_MASK ) <<3 )) | ( msg_id &
>ID_MASK );
>
>Lets suppose now the msg_type maps to MSG2_STATUS defined above
>
> switch (msg_type)
> {
> case _msg2_status: //got a status 2 message
>
> // double buffer it
> i = _msg2_message_index ^ 1;
> memcpy( &_msg2_message[i], buffer, sizeof(MSG2_STATUS) );
> _msg2_message_index ^= 1;
> _msg2_message_ready = TRUE;
> break;
>
> case _msg1_status: //got a status 1 message
>
> // double buffer it
> i = _msg1_message_index ^ 1;
> memcpy( &_msg1_message[i], buffer, sizeof(MSG1_STATUS) );
> _msg1_message_index ^= 1;
> _msg1_message_ready = TRUE;
> break;
> // and so on
> default:
> break;[/color]
The above code doesn't really do anything. Instead, you should
probably just add the message to a queue and signal a condition
variable on that queue so that another thread whose job is to process
these messages can do so.
[color=blue]
> }
> else
> {
> switch( msg_id )
> {
> case _msg2_header_error: //got a status 2 message
> _msg2_header_error_count++;
> printf (" msg2_header_error = &d ", msg2_header_error);
> break;
> case _msg1_header_error: //got a status 2 message
> _msg1_header_error_count++;
>
> // I suspect it'd be prudent to pass in the counters and
>have values returend by the ++ ostream approach
> printf (" msg1_header_error = &d ", msg1_header_error);
> break;
> // and so on
> default:
> break;
> }
> }
>
>So now, double buffer the data,increment a counter, then set a flag -
>for no errors. For errors, increment an error counter. The current
>template style approach is certainly more elegant[/color]
Yes, since all messages can be treated the same - no switch
statements. Polymorphism is the important thing here though -
templates are just an implementation detail.
[color=blue]
>A separate function within the class should retrieve the most current
>double buffer data.[/color]
Does this operate in the same thread? It might be good to put it in
another thread.
I suspect (assuming i understand templates) each[color=blue]
>buffered data will be 'pushed' into a container/holder/??[/color]
Seems like a good idea.
and when[color=blue]
>necessary (a 120hz processing rountine retrieves the latest) retrieved
>via a call to some function with the template parameter being the
>appropriate struct.[/color]
Yes, something like that. You could use the visitor pattern as Gianni
suggested.
[color=blue]
>Lastly, the individual structs form a part of an overall struct. ie
>struct OVERALL
>{
> MSG1_TYPE msg1;
> MSG2_TYPE msg2;
> lots more.
>}
>
>A separate function will retrieve the 'overall'.[/color]
Now I'm confused - aren't the messages separate? What *are* these
messages? What do they signify? Where do they come from? Some hardware
device? How are you meant to react to different messages? Why is there
more than one message type?
[color=blue]
>In essence, a template to 'get' the data and hold data for the
>requestor
>[color=green]
>>[color=darkred]
>> > CMDBase* Create(unsigned char const* buffer)
>> > {
>> > //MSG_HEADER* msg_id = reinterpret_cast<MSG_HEADER*>(buffer);
>> > MSG_HEADER* msg_id; // only here because reinterpret_cast error
>> >complaint
>> >
>> > //insert error checking!
>> > return (m_creators[msg_id->source])(buffer);
>> > }[/color]
>>
>> The above function should go more like this:
>>
>> CMDBase* Create(unsigned char const* buffer)
>> {
>> MSG_HEADER const* msg_id = reinterpret_cast<MSG_HEADER
>> const*>(buffer);
>> map_t::const_iterator i = m_creators.find(msg_id->source);[/color]
>
>map_t error ??[/color]
Whoops, that's meant to be a typedef for the map type that you need to
add to your class definition. e.g.
typedef std::map<int, Creator*> map_t;
Following is some complete compileable code that might help. It
basically shows how to enqueue all messages onto a queue, and how you
might go about processing those messages. If you might need different
processors for the messages in different circumstances, then the
visitor pattern might be appropriate. This is rather a lot to take in
if you are quite new to C++, but I hope it helps in any case.
#include <iostream>
#include <map>
#include <queue>
using namespace std;
//message queue
class CMDBase;
queue<CMDBase*> CMDQueue;
enum MSG_ID
{
msg1_cmd = 0,
msg1_status = 1,
msg2_status = 2
};
struct MSG_HEADER
{
MSG_ID source;
unsigned int msg_length : 12;
unsigned int count : 4;
};
struct MSG2_STATUS
{
MSG_HEADER header;
unsigned int idx : 1;
unsigned int jdx : 3;
unsigned int kdx : 4;
unsigned int spare : 24;
unsigned int ldx : 32;
};
struct MSG1_STATUS
{
MSG_HEADER header;
unsigned int idx : 1;
unsigned int jdx : 3;
unsigned int kdx : 4;
unsigned int spare : 8;
};
struct MSG1_CMD
{
MSG_HEADER header;
unsigned int idx : 1;
unsigned int jdx : 3;
};
class CMDProcessor
{
public:
void process(MSG2_STATUS& msg)
{
std::cout << "MSG2_STATUS processed\n";
}
void process(MSG1_STATUS& msg)
{
std::cout << "MSG1_STATUS processed\n";
}
void process(MSG1_CMD& cmd)
{
std::cout << "MSG1_CMD processed\n";
}
};
class CMDBase
{
public:
virtual ~CMDBase(){}
virtual void process(CMDProcessor* processor) = 0;
};
template <class CMD>
class CMDHolder: public CMDBase
{
public:
CMDHolder(unsigned char const* buffer)
{
memcpy(&m_CMDStruct, buffer, sizeof m_CMDStruct);
}
virtual void process(CMDProcessor* processor)
{
processor->process(m_CMDStruct);
}
private:
CMD m_CMDStruct;
};
class CMDFactory
{
public:
static CMDFactory& instance()
{
static CMDFactory fact;
return fact;
}
// Register the creator
template <class T> void Register(int Id)
{
m_creators[Id] = CMDCreator<CMDHolder<T> >;
}
// unregister creator
bool Unregister( const int& Id )
{
return m_creators.erase( Id ) == 1;
}
CMDBase* Create(unsigned char const* buffer)
{
MSG_HEADER const* msg_id = reinterpret_cast<MSG_HEADER
const*>(buffer);
map_t::const_iterator i = m_creators.find(msg_id->source);
if (i == m_creators.end())
{
throw runtime_error("Invalid source");
}
else
{
//call create function through pointer.
return (i->second)(buffer);
}
}
private:
typedef CMDBase* (*CMDCreator_t)(unsigned char const*);
template <class T>
static CMDBase* CMDCreator(unsigned char const* buffer)
{
return new T(buffer);
}
typedef std::map<int, CMDCreator_t> map_t;
map_t m_creators;
};
void register_types()
{
CMDFactory::instance().Register<MSG1_CMD>(0);
CMDFactory::instance().Register<MSG1_STATUS>(1);
CMDFactory::instance().Register<MSG2_STATUS>(2);
}
size_t ReadData(unsigned char* buffer, size_t size)
{
//generate sample data
static bool doMsg1 = false;
void* data;
size_t actualSize;
MSG1_STATUS msg1 = {};
MSG2_STATUS msg2 = {};
if (doMsg1)
{
msg1.header.source = msg1_status;
data = &msg1;
actualSize = sizeof msg1;
}
else
{
msg2.header.source = msg2_status;
data = &msg2;
actualSize = sizeof msg2;
}
if (size < actualSize)
throw runtime_error("buffer too short");
memcpy(buffer, data, actualSize);
doMsg1 = !doMsg1;
return actualSize;
}
CMDProcessor processor;
void processQueue()
{
while (!CMDQueue.empty())
{
CMDBase* cmd = CMDQueue.front();
CMDQueue.pop();
cmd->process(&processor);
delete cmd;
}
}
void fillQueue()
{
unsigned char buffer[512];
ReadData(buffer, 512);
CMDBase* newCommand = CMDFactory::instance().Create(buffer);
CMDQueue.push(newCommand);
}
int main ( void )
{
register_types();
for (int i = 0; i < 5; ++i)
{
fillQueue();
processQueue();
}
return EXIT_SUCCESS;
}