bugCERTI - Bugs: bug #24854, Protocol compatibility checks

 
 

bug #24854: Protocol compatibility checks

Submitter:  Petr Gotthard <gotthardp>
Submitted:  Mon 17 Nov 2008 10:02:30 AM UTC
   
 
Category:  CERTI Severity:  * 1 - Wish
Item Group:  Feature request Status:  In Progress
Privacy:  Public Assigned to:  gotthardp
Open/Closed:  Open Release:  * CVS
Reproducibility:  None Fixed Release:  * Unknown
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 14 Mar 2010 03:39:00 PM UTC, comment #10: 

No problem at all. ;-) I made all the proposed enhancement.

So, this is the current status:
CERTI_Message -- ready for test
CERTI_NetworkMessage -- waiting for the switch to the auto-generated protocol

Petr Gotthard <gotthardp>
Group Member
Sat 13 Mar 2010 06:58:50 PM UTC, comment #9: 

The algorithms looks good to me.
Some implementation remarks:

 - Do not update PrettyDebEx.cc this file is obsolete
   and should be deleted. In fact the functions defined
   here are not referenced anymore but I did forget to
   delete them.

   May be you can do it in 2 steps.
   1 delete the file and commit, then go on with the
   current patch.

 - I'd rather use stdint types like uint32_t and
   not "unsigned int", this is not a big deal
   but it'll be more easy to maintain and less
   error prone for 64bits types.

   static const unsigned int versionMajor
    --> 
   static const uint32_t versionMajor
  
 - It would be better to avoid std::cerr usage
   unless for preliminary debugging purpose.
   If the error leads to an exception you could
   attach the message as Exception reason using
   setException, e.g:

    answer->setException(e_RTIinternalError,"RTIA protocol Error")

   As usual if you need to build a more complicated
   message you can use a std::stringstream to build it
  
 - May be the anonymous enum
   enum {
        CONNECTION_PRELUDE, //!< Before OPEN_CONNEXION
        CONNECTION_READY,
        CONNECTION_FIN //!< After CLOSE_CONNEXION
    } _connection_state;

   could be named and documented

   /**
    * Add meaning full explanation for this ...
    */
   typedef enum ConnectionState {
        CONNECTION_PRELUDE, //!< Before OPEN_CONNEXION
        CONNECTION_READY,
        CONNECTION_FIN //!< After CLOSE_CONNEXION
   } ConnectionState_t

   ConnectionState_t _connection_state;
 
I'm sorry if I appear to be picky that time, but I think
that since more people get involved in the code we should
try to do our best to enhance the readability of the code
for newcomers.

If you are OK with my remark please update and commit.
If you are not may be we can go for a second
patch round :-)

Eric NOULARD <erk>
Group administrator
Sat 13 Mar 2010 03:56:24 PM UTC, comment #8: 

Here comes a proposed patch for the Federate--RTIA communication.

The mechanism is following:
 - OPEN_CONNEXION message must be the first message exchanged between federate and RTIA; currenly it's used for protocol version checks, but may be used for other purposes
 - the message definition file defines a major and a minor version number
 - both peers must use the same major number; if major numbers are different, an error is reported
 - each pair negotiates an "effective minor number", which is the lower of the two minor version numbers (federate and RTIA); it's currently not used but may be useful to achieve backward compatibility

What do you think?

(file #19933)

Petr Gotthard <gotthardp>
Group Member
Fri 12 Mar 2010 09:53:27 PM UTC, comment #7: 

I won't finally manage to check-in my changes to
the NetwrokMessage layer until next week.
The changes are bigger than expected.

Do not wait for my change unless you want to wait one more
week before that.

Eric NOULARD <erk>
Group administrator
Mon 08 Mar 2010 08:45:19 AM UTC, comment #6: 

Well, I guess I should propose something since I've created the issue. ;-) I will try to develop something this weekend.

Petr Gotthard <gotthardp>
Group Member
Sun 07 Mar 2010 09:40:06 PM UTC, comment #5: 

Agreed.

Would you have time/are you willing to propose a patch for this?

If you do so, please checkout/update an up-to-date tree since there has been a lot of clean-up lately.
(And I intend to pursue this effort).

We do not currently have any "open connexion" message just the "close connexion" one.

If you add this new message, please add its
spec accordingly in

CERTI_Message.msg
and
CERTI_NetworkMessage.msg.

Currently the generator should work just fine for RTIA Messages:

cd <path-to-certi>/scripts
./GenerateMessages.py --input=CERTI_Message.msg --language=CXXCERTIMessage --type=body --output=../libCERTI/M_Classes.cc

./GenerateMessages.py --input=CERTI_Message.msg --language=CXXCERTIMessage --type=header --output=../libCERTI/M_Classes.hh

should produce updated files ready to compile without
any manual editing.

Concerning RTIG Message (aka Network Messages) the generator needs some work in order to re-create the NM_Classes.[hh|cc] without manual editing needs.

I'll try to update the Generator for this soon.

Eric NOULARD <erk>
Group administrator
Sun 07 Mar 2010 07:50:25 PM UTC, comment #4: 

My idea was to put a version number into the CONNEXION messages and send some user-friendly message if the peer is not compatible. Currently, if you try CERTI 3.4 against 3.3.x (or vice versa?), you get a very ugly message.
I thought we could have OPEN_CONNEXION messages that can be used for connection initialization and transmission of various initialization parameters, including the version number.

The option (2) seems to me unnecessary. If we are careful about increasing the protocol numbers (we could a major and minor version numbers) and keep maximal backwards compatibility, perhaps we don't need version number in every single message.

Petr Gotthard <gotthardp>
Group Member
Sun 07 Mar 2010 06:08:30 PM UTC, comment #3: 

Hi Petr,

You are pretty right.
Do you have some design idea in mind?

The last time I thought about that I had two ideas:

1) Add an initial CERTI_VERSION message sent from
   Federate --> RTIA --> RTIG

   This way we can at least forbid incompatible
   libRTI/RTIA and/or RTIA/RTIG couple to try to work together

2) Add a version field in CERTI ([Network]Message) in order
   to be able to handle "different version" of federate.

   libCERTI would have to maintain "compatible" message
   in order to be able to send appropriate message content
   depending on the CERTI version discovered at the other end.


These are "rough" idea and that need more detailed design
before trying to implement it. Any help is welcomed.

Eric NOULARD <erk>
Group administrator
Sun 07 Mar 2010 02:21:47 PM UTC, comment #2: 

Let me bring this up again: since the CERTI network protocols are now being redesigned, isn't now the best time to implement some protocol version-ing and version checks?

Petr Gotthard <gotthardp>
Group Member
Mon 17 Nov 2008 02:00:27 PM UTC, comment #1: 

I totally agree we should AT LEAST
add a CHECK_VERSION network message
which would be the first message automatically
sent to RTIG byt RTIA when RTIambassador is created.

The first basic implementation should
make the RTIG refuse a federate using a
different version of libCERTI by sending
an appropriate error message in the
CHECK_VERSION answer.

Then we should design and implement a compatibilty mode...
ideally a version n RTIG should be able to handle
a version p federate if p < n.



Eric NOULARD <erk>
Group administrator
Mon 17 Nov 2008 10:02:30 AM UTC, original submission:  

The CERTI protocol changes and RTIA/RTIG/federates running different CERTI versions are very often incompatible.
To avoid incompatibility problems a geographically distributed environment, introducing protocol versioning and compatibility checks is very critical.

Petr Gotthard <gotthardp>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #19933:  versioning.patch added by gotthardp (14KiB - text/x-patch - Proposed patch.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by erk (Posted a comment)
  • -email is unavailable- added by gotthardp (Submitted the item)
  •  

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-03-13 gotthardp Attached File- Added versioning.patch, #19933
        StatusNone In Progress
    2010-03-08 gotthardp Assigned toNone gotthardp

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code