NMEA2000 -> AIS Discussion for Library Addendum

[ronzeiller]

As beginning to discuss on https://github.com/ttlappalainen/NMEA2000/pull/153 We would need some kind of lookup table for different EPFD Fix Types on NMEA2000 and AIS:

Edit 7.11.2019: Lookup Table for Encoding AIS Fix Type as discussed below

Name	int N2k	int AIS	remarks
Undefined	n/a	0
GPS	0	1
GLONASS	1	2
GPS/GLONASS	2	3
Loran-C	n/a	4	never sent, since Loran-C is off since years :-)
GPSSBASWAAS	3	1	n/a in AIS, but it is a GPS....
GPSSBASWAASGLONASS	4	1	same as 3.)
Chayka	5	5
Integrated	6	6
Surveyed	7	7
Galileo	8	8
Internal GNSS		15	?????? we do not get from N2k yet
Undefined	254	0	new Type in N2k Library
Not available	255	0	new Type in N2k Library

[ttlappalainen]

I thought something like that too. One problem is that Undefined is not NA. Other problem is that rest of unlisted values are reserved for future use. Where did you got "Internal GNSS"?

So if we would have on tN2kGNSStype ..._Unavailable=255, ..._Undefined=254, then in AIS messages you could translate value back and forward according your table (with 3->1, 4->3, 254->0).

If we would use that, according to my tests it would be fastest to do with switch. I wrote test with simple switch or having vector and looping with linear for, switch was always faster. Sequential search would be naturally fastest, but is more complex with PROGMEM.

[ronzeiller]

https://www.navcen.uscg.gov/?pageName=AISMessagesAStatic

0 = undefined (default) 1 = GPS 2 = GLONASS 3 = combined GPS/GLONASS 4 = Loran-C 5 = Chayka 6 = integrated navigation system 7 = surveyed 8 = Galileo, 9-14 = not used 15 = internal GNSS

switch is fine for me.

I think, as in AIS it is not differentiated between unavailable and undefined, everything which is not valid will be translated into 0. In a standalone AIS Encoder validation of incoming values would be made to 0.....15.

But here, as we get datas exclusively from N2k library, I would do the check on the values coming from your Parser 0....15 before translation. Or do you think as they are checked in the parser a second check would not be necessary?

The question is, which values could be on N2k-bus? Not 254 nor 255, so why invent them?

Apropos "not available" For example DESTINATION (entered by human, manually) could be "not set" . From the N2kParser we would get 0 (empty string). This would then be translated into @@@@@@@@@@@@@@@@@@@@ (20 characters) as it stands for "not available" Same with Call sign and Name....

[ttlappalainen]

For backward compatibility you can not change current tN2kGNSStype. So we do not have undefined there and we can not put it below 15, since they are reserved and may get values in future. Mixing things is not easy.

I did not got you point about "not available". Isn't empty string as not available? If you have data A@@@@@@@@@@@@@@@@@@@ then name is A. If all are @, then name is empty.

[ronzeiller]

For backward compatibility you can not change current tN2kGNSStype.

This was not my intention.

So we do not have undefined there

Then it is very easy. If we do not have "undefined" in N2k means, that we will never have Zero (undefined) for that in AIS, because the encoder cannot get wrong numbers from the Parser and obviously "undefined" does not exist on N2k in this matter.

And now I start to understand your idea: If you cannot decode the value from N2k then you will hand over e.g. 255 to the Parser. And if the encoder gets 255 out of the Parser, then it will write Zero for undefined. 👍 As both will be Zero then, I do not think that we need 255 and 254. But anyway it is just another case.

case 254:
case 255: 
       x= 0; 
      break;

I did not got you point about "not available". Isn't empty string as not available? If you have data A@@@@@@@@@@@@@@@@@@@ then name is A. If all are @, then name is empty.

Yes it is.

What I meant is: for AIS it makes probably no difference if something is "not available" or is "undefined". It will be empty (Strings) or the default value, like SOG=1023, ROT=128, COG=3600 and so on. (same as above for 254 and 255, both 0)

[ttlappalainen]

Normally highest value means NA, thats why I mentioned 254 for undefined and 255 for NA. NA does not currenly have use, since on N2k side there is no NA for GNS type. But it is like a spare.

Do you mean that they have SOG as default 1023 on N2k side? If they have that kind of defaults on 0183 side, they should be translated to NA on N2k side.

[ronzeiller]

Normally highest value means NA, thats why I mentioned 254 for undefined and 255 for NA. NA does not currenly have use, since on N2k side there is no NA for GNS type. But it is like a spare. 👍

Do you mean that they have SOG as default 1023 on N2k side? If they have that kind of defaults on 0183 side, they should be translated to NA on N2k side.

No idea what they send on N2k. The values I mentioned are "default" in AIS sentences.

You are right of course, an AIS-NMEA->N2k converter should translated such values to NA. But, do you think that anybody would ever need or use an AIS-NMEA->N2k converter? What could be the use of this?

[ttlappalainen]

I have old transmitting AIS. If I would have also new plotter, I would like to have only N2k and AIS on plotter. I think that I won't ever have plotter, since I use OpenCPN, but some other could probably have above case.

In N2k side NA should be handled right, if you use right typefor reading and writing and e.g. N2kDoubleNA definition. If you write NMEA0183 parser function, I prefer to translate default values to NA

[ronzeiller]

If you write NMEA0183 parser function, I prefer to translate default values to NA

Yes, this would be the best 👍

Should we open new issues for each point arises? Or should we keep all AIS discussions together in one issue?

Here is a new one: Do we have actual date and time as global var somewhere around in the library to calculate ETA Date from Days since January 1, 1970 and ETA Time from Seconds since midnight which we get from ParseN2kPGN129794?

[ttlappalainen]

I prefer one common AIS discussion. You could open new for that and add link here.

Library is core for handling NMEA2000 data. It can not have that kind of global variable, since library may handle just device, which has nothing to do with GPS. N2kMessages helps for common messages for beginners. I use just core and different way for message descriptions. For some things I use N2kMessages. So over the whole library you have to write top program for handling data. Message handlers on library should just set or parse messages and not use any global variables.

[ronzeiller]

For multi parted NMEA, how could this be solved?

At the moment I have an ugly workaround: Making the first NMEA Message part in:

std::string SetAISClassAMessage5(NMEA0183Msg, AISMsg) {

  if ( !NMEA0183Msg.Init("VDM","AI", Prefix) ) return 0;
  // build together part one.....

 return messagePart2;
}

Returning the second part of message and in the calling function I have:

`void tN2kDataToNMEA0183::HandleAISClassAMessage5(const tN2kMsg &N2kMsg) {

   // do some stuff....

SendMessage(NMEA0183Msg);    // sending the first part created at SetAISClassAMessage5()

// build the second NMEA Message part 

SendMessage(NMEA0183Msg);    // sending the second part
}

How could be build two "NMEA" sentences which are sent together?

Would it be possible to SendMessage(NMEA0183Msg) somehow within the SetAISClassAMessage5() function?

[ttlappalainen]

Also may be better to collect AIS messages to own modules and separate them from N2kMessages.

If both parts are independent messages, above is logically right way.

Be aware not to do something, which causes libraries to join together. The problem starts, if you create new class tAISMsg, which both library uses. Then you can not have that definition on both or one one, since that would then glue other to other, which is not good. Both libraries should work independent. So the steps should be thought carefully to avoid extra headache for current library users.

One possiblitity is that there is on both side class, which handles message as I sent as example on other issue. Then your converter copies data between them. In this way, current library users can use new messages without need for other library.

[ronzeiller]

Well, I must confess, that my C++ knowledge is far away from this to write something you are talking about.

The status quo is: I have an AIS library running, which is a standalone part. It is no standalone converter. It is an addon which of course fully depends on NMEA2000 and NMEA0183 library. It integrates seamlessly and easy for users, without any changes for current library users.

It is very simple. If you want AIS-NMEA on Serial or on WiFi you include two files and call a function within a N2kPGNHandler, (like in your example N2kDataToNMEA0183) that´s all.

When everything is tidied up a little bit I will put it on github. (I am sure that there will be much to improve then and I highly appreciate all your tipps!) But, I do not believe, that this will ever meet your standards, so this will probably never be a pull request or part of the official NMEA2000 or NMEA0183 library. ;-)

At the moment AIS Message Type 1 and 5 are converted into NMEA and sent. Currently I am watching many boats (AIS targets) in iSailor from a logfile I wrote at my last regatta. Teensy Logfile Reader writes to --> N2k Bus --> ESP32 reads N2k, converts datas to NMEA0183 and AIS-NMEA --> ESP32 send datas over WiFi --> iPad iSailor nav software

[ttlappalainen]

That is other and good enough way to make it. The only drawback with that is that then both libraries does not offer way to parse AIS data to "clear data". So if I would like to e.g. just track ships on NMEA2000, library does not offer class or function for it. An other question is that how many needs it or is it simple to copy-paste part of your code (if license allows) to own code. N2kMessages module works for making simple things.

The only way to learn programming is to write code, learn from others code and always try to make next code better.

[ronzeiller]

That is other and good enough way to make it. The only drawback with that is that then both libraries does not offer way to parse AIS data to "clear data". So if I would like to e.g. just track ships on NMEA2000, library does not offer class or function for it.

If needed this could be done later quite similar your getNMEA / setNMEA thing. For incoming NMEA0183 you have the same handle thing, if I remember right.

An other question is that how many needs it or is it simple to copy-paste part of your code (if license allows) to own code. N2kMessages module works for making simple things.

It is the same license as in your library. People could copy code, but it will be easier to just include it as it is.

The only way to learn programming is to write code, learn from others code and always try to make next code better.

👍 It is similar to building a house, the first one is for an enemy, the second is for a friend and the third you can use for yourself :-)

The thing is, that there are not so many Encoder out there and I do hope, that people will give feedback and help improving the code. Also, I think our discussion on principles of AIS will improve or let´s say prepare things for your code, if someone will program something in this matter.

[ronzeiller]

New thing we have here You have:

enum tN2kAISNavStatus {
                            N2kaisns_Under_Way_Motoring=0,
                            N2kaisns_At_Anchor=1,
                            N2kaisns_Not_Under_Command=2,
                            N2kaisns_Restricted_Manoeuverability=3,
                            N2kaisns_Constrained_By_Draught=4,
                            N2kaisns_Moored=5,
                            N2kaisns_Aground=6,
                            N2kaisns_Fishing=7,
                            N2kaisns_Under_Way_Sailing=8,
                            N2kaisns_Hazardous_Material_High_Speed=9,
                            N2kaisns_Hazardous_Material_Wing_In_Ground=10,
                            N2kaisns_AIS_SART=14,
                          };

For AIS we would need also 15 for undefined which is default.

[ttlappalainen]

Isn't there also vessel type and some other definitions? There could be N2kAISTypes.h for those definitions, if there will be lot of new definitions. The effect is only for handling. It does not change anything for performance.

[ronzeiller]

As far as I saw, that should be the last "missing" one for AIS Message Types 1/3 and 5. Those are the most common and mostly needed Message Types?

A N2kAISTypes.h could may be of interest, when parsing AIS-NMEA -> N2k will be made? For N2k -> AIS-NMEA I do not need own types. I have to take your types from N2kTypes.h anyway for the parser. After Parsing the values get validated and/or "translated" into (most of them) uint8_t, and then converted into 6bit binary in one function for each field.

And, if there is something not defined in N2k, then this value will never come through to the Parser, right? Therefore tN2kAISNavStatus=15 will never be needed for N2k->AIS converter. In addNavStatus(N2kAISNavStatus) I firstly check: (N2kAISNavStatus >= 0 && <= 14)? status = N2kAISNavStatus : status = 15;

Checks within AIS allowed values like this will automatically cover your NA values.

BTW: started to rewrite everything with char arrays instead of std::string ;-) It will be then more or less similar to your NMEA0183Msg and NMEA0183Messages style then...

[ttlappalainen]

Be carefull with char arrays. String is safe and you will not write behind. With char arrays it is better to use xn functions like strncpy. snprintf is easy for conversions and does not write behind array.

Then one trick to fill char array is to add data to it: void PassToBufferEnd(char &buf, size_t &MaxLen) { size_t len=strlen(buf); buf+=len; MaxLen-=len; } ... size_t MaxLen=200; char Result[200]; char buf=Result;

snprintf(buf,MaxLen,"%.3f,",Val1); PassToBufferEnd(buf,MaxLen); snprintf(buf,MaxLen,"%.3f,",Val2); PassToBufferEnd(buf,MaxLen); ...

Now I do not remember your environment and platform. printf does not work in all systems for floats. I use PlatformIO and Teensy, where it works fine. Natrurally you can make simple functions, which uses ftoa or some other functions: vois AddToBuf(double val; char *&buf, size_t &MaxLen) {...}

[ronzeiller]

Now I do not remember your environment and platform.

Hackintosh, Atom + PlatformIO, normally Teensy, but now trying ESP32. Could be nice with Wifi integrated. Have two devices and both make problems after a while. Rebooting and WiFi information is seen on Serial, but afterwards nothing happens anymore. Reads nothing from N2k-Bus. Sometimes it starts working again, after rebooting/unplug/plugin/switch to another USB, etc. Working for hours with many uploads and then after restart suddenly nothing. Tried it with 3 different types of CAN transceivers (2562, 2551, TJA1050), no difference. Quite annoying....

[ronzeiller]

2-parted N2k PGNs Do you have any ideas what to do with 2-parted N2k AIS Messages? (PGN 12809 + 12810) *) Global static List, where adding datas for a specific UserID (MMSI). Store there incoming datas for a while, deleting them after 5 Minutes or after 5-10 new messages from other boats? They should come close together or not?

*) Just saw that quite many small crafts send this PGNs

[ttlappalainen]

One thing with ESP32 is that you need good power to use WiFi. We have 0.5 A PSU feeding ESP32 and just on power pin of ESP32 470 uA capacitor. Otherwise it rebooted itself by WiFi transmitting. In isolated systems we use ISO1050 and on those, which does not require isolation (=not connected any other devices or connection is isolated) MCP2562. Have not seen problems with communcation. But to get everything work, I passed default WiFi library.

2-Parted PGNs. That depends how they are handled on the NMEA0183 side. If you can not forward data from one PGN to NMEA0183, then only solution is to store information. One way is to build system to handle them. Lest think you have tShip structure to hold ship information. Then you could have vector of pointers to those structures tShip Ships=new (tShip*)[100]; Vector will be initialized as all 0. So now, when you need new to add new ship, you search from first free ship "slot", If there is none, create new tShip and add pointer to vector. When time out for ship, you mark tShip free, but do not delete it so you can reuse tShip and avoid new/delete fragmentation. If you reach more than 100 ships, then at that point you just need to create new vector with some extra ship expectation, copy old pointers and delete old vector (this is how many container classes like string works). Naturally you can use std:vector class to save that part. Now now if you do not store any other data of ships, you can fire NME0183 sending on PGN129810 reception and mark tShip as free for reuse. Also you one poller, which goes through all ships and marks them free on timeout.

[ronzeiller]

Hi Timo, the first "release" of the AIS add on for your NMEA2000/NMEA0183 Library is ready https://github.com/ronzeiller/NMEA0183-AIS

There is no need to change anything in your libraries. Basically the Add On is building an AIS Payload which can be loaded to build a NMEA0183 sentence like this example for Message Type 18:

if ( !NMEA0183Msg.Init("VDM","AI", _Prefix) ) return;
if ( !NMEA0183Msg.AddStrField("1") ) return;
if ( !NMEA0183Msg.AddStrField("1") ) return;
if ( !NMEA0183Msg.AddEmptyField() ) return;
if ( !NMEA0183Msg.AddStrField(AISClass) ) return;
if ( !NMEA0183Msg.AddStrField( AISMsg.GetPayload() ) ) return;
if ( !NMEA0183Msg.AddStrField("0") ) return;    

SendMessage(NMEA0183Msg);

AIS Message Types 1, 5, 18 and 24 are written. Certainly many things to could be improved. So, your recommendations are highly appreciated.

[ttlappalainen]

Hi,

First layout hint. I prefer to use two empty space for each indent. Now I think that you have used tab, but your editor has not converted them to spaces. If you go to read code on GitHub, it is unequally indented. Editors should have setting "Convert tab to x spaces" and set x to 2. Some editors also has setting to remove empty spaces from end, which is good also.

Then techical question. Is there any sense to use tAISMsg as alone for any other purpose? The point is that is it anyway always as part of NMEA0183 message? If so, then tAISMsg could be inherited from tNMEA0183Msg. In this way e.g. SetAISClassABMessage1 would also do if ( !NMEA0183Msg.Init("VDM","AI", _Prefix) ) return; ...

Then the main code would look like if ( SetAISClassABMessage1 (AISMsg,...) ) { SendMessage(AISMsg); }

So the normal inheritance question: "is tAISMsg tNMEA0183Msg". If answer is yes, then you could use inheritance and put some code under set function.

Did I sent you example of making class from each PGN? That would work here, but that we can look later.

[ronzeiller]

First layout hint. I prefer to use two empty space for each indent. Now I think that you have used tab,....

Thanks for the hint. pushed within CLI without watching the result on Github.... Should be corrected now and Atom Editor set to "soft Tab"

Then techical question. Is there any sense to use tAISMsg as alone for any other purpose?

No, not at all. Great input!! 👍 Now it is a clean and tidy child class of NMEA0183Msg

Did I sent you example of making class from each PGN? That would work here, but that we can look later.

You did send it, thank you. In fact I started with something which ends up in:

if ( SetAISClassABMessage1 (AISMsg,...) ) {
SendMessage(AISMsg);
}

But then I could not find a way to send two messages (two parted AIS sentences), as SendMessage(....) did not work in the SetAISClass.... function.

Do you mean this could solve the SendMessage() problem?

[ttlappalainen]

If you inherit class tNMEA0183Msg : public tNMEA0183AISMsg { you can use all tNMEA0183Msg public and protected methods and properties.

For two part message case you build part1 as default on SetAISClassAMessage5 Then you have two other methods const tNMEA0183AISMsg& BuildPart1(); const tNMEA0183AISMsg& BuildPart2(); In both you do tNMEA0183Msg rebild and return reference to itself. You can also keep flag, which part is currently build up, so that repeated call to e.g. BuildPart1() doe not rebuild message. So now you get: if ( SetAISClassBMessage24(AISMsg,...) { SendMessage(AISMsg.BuildPart1()); SendMessage(AISMsg.BuildPart2()); }

The first call could be just SendMessage(AISMsg); but sometime it is better to use explanative way.

[ttlappalainen]

Some more:

There is invalid comment on (check also others) //***** // 129039 AIS Class B Position Report --> AIS Message Type 5: Static and Voyage Related Data void tN2kDataToNMEA0183::HandleAISClassAMessage5(const tN2kMsg &N2kMsg) {...

As above I prefer to use tNMEA0183AISMsg. It is a bit longer, but more self explaining and makes it clear that it is NMEA0183

On AISMessages.h you have:

ifndef Arduino

typedef uint8_t byte;

endif

Where byte is needed? Also it would be better to use uint8_t, when you mean 8-bit unsigned integer. Byte is not defined in c++. Also I should fix some things on NMEA2000.

On AISMessages.h you have: // **** Helper for AIS *** bool AddMessageType(tAISMsg &AISMsg, uint8_t MessageType); ... If those functions has not been used anywhere except within AISMessages.cpp, it is better to define them just in the beginning of AISMessages.cpp as static bool AddMessageType(tAISMsg &AISMsg, uint8_t MessageType); With this you avoid possible conflict if someone decides to use same names like AddSeconds in his own code. If you need them outside, then you could make all your functions within namespace: namespace NMEA0183AIS { bool SetClassABMessage1(...); ... bool AddETADateTime(...) ... }; Then you refer to those by using NMEA0183AIS::SetClassABMessage1(...). Note that inside .cpp you also use namespace NMEA0183AIS { at beginning and inside it you do not need to use NMEA0183AIS:: for namespace functions. But you do not need necessarily namespace, if thos functions won't be needed elsewhere.

Also AISMessages.h/.cpp could be NMEA0183AISMessages.h/.cpp.

[ronzeiller]

Thank you for your patience and guiding!

There is invalid comment on (check also others)

this and the Tab issue should be corrected now

If you inherit class tNMEA0183Msg : public tNMEA0183AISMsg { you can use all tNMEA0183Msg public and protected methods and properties.

Just for my understanding: shouldn't it be? class tNMEA0183AISMsg : public tNMEA0183Msg {

If those functions has not been used anywhere except within AISMessages.cpp, it is better to define them just in the beginning of AISMessages.cpp as static bool AddMessageType(tAISMsg &AISMsg, uint8_t MessageType);

Done

As above I prefer to use tNMEA0183AISMsg. It is a bit longer, but more self explaining and makes it clear that it is NMEA0183

Done

[ronzeiller]

For two part message case you build part1 as default on SetAISClassAMessage5 Then you have two other methods const tNMEA0183AISMsg& BuildPart1(); const tNMEA0183AISMsg& BuildPart2(); In both you do tNMEA0183Msg rebild and return reference to itself. You can also keep flag, which part is currently build up, so that repeated call to e.g. BuildPart1() doe not rebuild message. So now you get: if ( SetAISClassBMessage24(NMEA0183AISMsg,...) { SendMessage(NMEA0183AISMsg.BuildMsg24PartA()); SendMessage(NMEA0183AISMsg.BuildMsg24PartB()); }

Sorry, I have no idea, where to put

const tNMEA0183AISMsg& BuildMsg24PartA();
const tNMEA0183AISMsg& BuildMsg24PartB();

And where I should put the functions itself?

Thank you!

[ttlappalainen]

Just checked are you awake with definition class tNMEA0183Msg : public tNMEA0183AISMsg

I think this would be best to change totally classes. I'll explain more tomorrow.

[ronzeiller]

I think I got it now

In NMEA0183AISMsg.h

const tNMEA0183AISMsg& BuildMsg5Part1();
const tNMEA0183AISMsg& BuildMsg5Part2();
const tNMEA0183AISMsg& BuildMsg24PartA();
const tNMEA0183AISMsg& BuildMsg24PartB();

and SendMessage( NMEA0183AISMsg.BuildMsg5Part2() ); etc. in the if ( SetAISClassBMessage5(NMEA0183AISMsg,...) {

Please see branch develop1 for that.

[ttlappalainen]

I thought a new idea. Even tNMEA0183AISMsg is tNMEA0183Msg, it is not good to inherit it. Inheriting it brings also all methods like AddDoubleField available for tNMEA0183AISMsg. But since AIS message has to constructed in special way, it is not good to allow user anymore to use AddDoubleField and other methods, which will destroy AIS message. I have to blame my bad design for tNMEA0183Msg.

So instead of inheritance you could do:

class tNMEA0183AISMsg {
protected: 
  tNMEA0183Msg Msg;
 // add other protected members like payload etc.
protected:
  virtual void BuildMsg()=0;
  // Generally Used
  bool AddIntToPayloadBin(int32_t ival, uint16_t countBits);
  bool AddBool(bool &bval, uint8_t size);
  ...
  // ************************  Helper for AIS  ***********************************
  bool AddMessageType(uint8_t MessageType);
  bool AddRepeat(uint8_t Repeat);
  bool AddUserID(uint32_t UserID);
...
public:
...
  operator const tNMEA0183Msg&()  { BuildMsg(); return Msg; }
...
};

So tNMEA0183AISMsg is just common AIS message class. Then you inherit each different AIS message type from that:

class tNMEA0183AISClassABMessage1 : public tNMEA0183AISMsg {
protected:
  virtual void BuildMsg();
...
public:
...
  bool SetLatitude(double Heading);
...
  bool SetHeading(double Heading);
...
}

So now you conversion handler will change to:

if ( ParseN2kPGN129038(N2kMsg... ) ) {
  tNMEA0183AISClassABMessage1 NMEA0183AISMsg;
  NMEA0183AISMsg.SetLatitude(Latitude);
  NMEA0183AISMsg.SetLongitude(Longitude);
...
  NMEA0183AISMsg.SetHeading(Heading);
  NMEA0183.SendMsg(NMEA0183AISMsg);
}

The most important thing with making them classes like this you can hide all unnecessary methods from user. E.g. there is no sense have AddMessageType public, since it is 1 for tNMEA0183AISClassABMessage1 and 5 for tNMEA0183AISClassAMessage5 etc. Other idea of classes is that you provide only those methods for user, which makes sense for that class.

So what happens now on SendMsg. Operator operator const tNMEA0183Msg&() from base class tNMEA0183AISMsg will be called, which first calls virtual method BuildMsg. When you write that virtual message for each new class, that will be called for that specific class. For two part messages you can make it work so that first time it will build part 1 and for second call part 2. So calling

NMEA0183.SendMsg(NMEA0183AISMsg);  // Send part1
NMEA0183.SendMsg(NMEA0183AISMsg);  // Send part2

Would send first part 1 and then part 2. You can also have methods GetPart1() and GetPart2() and use

NMEA0183.SendMsg(NMEA0183AISMsg.GetPart1());
NMEA0183.SendMsg(NMEA0183AISMsg.GetPart2());

With this way you would only have all classes in NMEA0183AISMsg.h/.cpp and NMEA0183AISMessages.h/.cpp would disappear. The changes would be mostly just move code logic from one place to another.

Did you got the point?

[ronzeiller]

I think I got the point and the pros to do it like this

Inheriting it brings also all methods like AddDoubleField available for tNMEA0183AISMsg. But since AIS message has to constructed in special way, it is not good to allow user anymore to use AddDoubleField and other methods, which will destroy AIS message.

This is true, but one point is: Now everything is strait forward and relatively similar in a form, the user knows from making NMEA0183 sentences. To do it the new way make things more complicated and heavy to read the code for every new user.

So now you conversion handler will change to:

if ( ParseN2kPGN129038(N2kMsg... ) ) {
tNMEA0183AISClassABMessage1 NMEA0183AISMsg;
NMEA0183AISMsg.SetLatitude(Latitude);
NMEA0183AISMsg.SetLongitude(Longitude);
...
NMEA0183AISMsg.SetHeading(Heading);
NMEA0183.SendMsg(NMEA0183AISMsg);
 }

With the new class(es) a user must copy more details to exactly build a correct AIS sentence. Now it is easy, because everything is made by the NMEA0183AISMessages Class, just calling SetAISClassBMessage18(....) as blackbox.

Secondly, a user can also destroy an AIS sentence very easily by using wrong NMEA0183AIS methods in the new class. And still a lot of methods (like ETA Time Building) are used from the NMEA0183Msg class.

And finally I sometimes think, that too much OOP makes code theoretically perfect, but makes it unnecessarily complicated and non-transparent for every user. (....shouldn't then also be each class for each AIS sentence in an own file....?)

.....is just, that I do not know if it is really worth doing it the new way, or may be I am just not confident doing this by myself.

But perhaps you also have already the other way around (Get NMEA0184AIS and convert them to N2k) in mind, where this changes would be essential too?

[ttlappalainen]

With the new class(es) a user must copy more details to exactly build a correct AIS sentence. Now it is easy, because everything is made by the NMEA0183AISMessages Class, just calling SetAISClassBMessage18(....) as blackbox.

There is no difference on calling single Set function or setting each one by one. The biggest difference is that with class, you can construct message data as defaut to all not available value and then just set e.g. lat and lon, if you do not have other data available. Note that you can also offer method like: NMEA0183AISMsg.Set(Latitude,Longitude,Heading,...);

Secondly, a user can also destroy an AIS sentence very easily by using wrong NMEA0183AIS methods in the new class.

Here you have some misunderstanging. With right classes you publish only methods, what user is allowed to use. So he should not able to be mesh it at all. This works, if it is possible to set "properties" like lat, lon in any order. But with quick look to code they were just in specific position on payload, so order does not matter.

.....is just, that I do not know if it is really worth doing it the new way, or may be I am just not confident doing this by myself.

Two things: I am helping and you would learn new things. You could leave current as it is and create new try. Then use my direct email, so we can develop it and publish, when it looks good.

But perhaps you also have already the other way around (Get NMEA0184AIS and convert them to N2k) in mind, where this changes would be essential too?

That is something I am thinking background.