Use of checks for NA data in PGN130311 ?
dagnall53 opened this issue · 5 comments
Hi, This is a general note, that turned out to be a bit of a 'rabbit hole' I would appreciate your comments!
I spotted that a user who sent me some Debug data on a different topic had a N2000 "source 3" sensor that sends sends "environmental data" as well as Water Speed / depth and Log. - and this was reporting a Silly MDA converted equivalent.
He and I were both surprised by this as he did not know he had this baro sensor!
N2K: PGN:130311 SRC:3 = [031FD07] (Environmental Parameters)
N2K>XXX:$IIMDA,-295299.83,I,-10000.000,B,-1000000273.1,C,,,,,,,,,,,,,,*33
Tracing, I noted that the ParseN2kPGN130311 does not contain any explicit checks on the data values (no check for NA).
But I see that you use ( if (!N2KisNA(...)) in the pre-set range conversions :
eg.. inline double mBarToPascal(double v) { return N2kIsNA(v)?v:v*100L; }
However, I note that ParseN2kPGN130311 perhaps dates from before these range conversion functions and uses a direct(?) conversion?:
AtmosphericPressure=N2kMsg.Get2ByteUDouble(100,Index);
So: I was wondering if rather than the "100,Index" it should use mBarToPascal (or PascalTomBar) somehow ??
But I ran out of competence trying to figure the correct formulation.
I saw you last looked at these (admittedly deprecated) environmental PGN was in 2016? so this is obviously not something that is causing anyone any real problems!
Many thanks.
Dagnall
Data should be always SI, there are some exceptions - sorry for that. Pressure SI unit is Pa. So when you get data from one source and send it to other source, you should do the checks.
N2kMsg.Get2ByteUDouble(100,Index); returns NA (ValueDoubleNA) in case of field has NA value. Thats also why it is important to use Get2ByteUDouble instead of Get2ByteDouble, since their field NA differs. So for 130311 field 6 you get either NA or value in Pa. If you feed that to MDA, you need to use NMEA2000 library PascalTomBar(v) as input, which does not do conversion, if value is NA.
And about history you may wonder why there is ValueDoubleNA and not nan. At very beginning I had problems on Arduino environment with nan so I decided to use simple not realistic value. But users should take care and use IsNAValue(v) for values returned from library. Today we could probably switch it to nan, but better not to touch tested system.
Many thanks. That will be a useful reference!
I decided to add a check for ValueDoubleNA in my very crude 130311 to MDA "HandleEnvironmentalParameters"
(attached in case it is of any use, - but embarrassingly crude compared to your code)
Arguably I should not bother converting Atmospheric pressure from a deprecated sensor, but I think I saw it (PGN130311) somewhere and so added it when I was trying to learn how do do conversions..
Some hints:
- Do not compare to N2kDoubleNA. Use IsNAValue(v). Then it is possible to change N2kDoubleNA to nan - even I doubt ever to do it. Actually you even do not need it as you see below.
- With tNMEA0183Msg::AddDoubleField you can use multiplier. Now you have set it to 1. By using multiplier, function checks NA and you do not need to check NA in your code.
NMEA0183Msg.AddDoubleField(AtmosphericPressure,PaToIHg,"%.2f","I" ) - Other cases I prefer to use function PascalTomBar, KelvinToC, PascalToPSI etc. instead of using simple multiplication. Functions can then do NA check and return NA. NMEA0183 functions just sets empty ('') for NA.
- You can add also value and unit at same for NA values. Then you do not need to use explicit AddStrField.
tNMEA0183Msg::AddDoubleField(NMEA0183DoubleNA,1,DefDoubleFormat,"T"); - Internally in program it is is always best to only SI. Unfortunately NMEA2000 library does not do it, but if I would write it now even lat/lon would be internally radians. Calculations are easier, when you always use SI. In principle it does not matter, what it is as soon you always use same. But then problem is that between different libraries same variates, so I always first convert data from any library to SI. Simple example is flow and time. In NMEA2000 library flow is l/h. If that would be properly m3/s and you would have time just s, then you get volume simply with v=flow*time and if you like to show it you do converion you like m3ToLitres(v).
As conclusion you can clean your code to:
tNMEA0183Msg NMEA0183Msg;
if ( !NMEA0183Msg.Init("MDA", "II") ) return;
if ( !NMEA0183Msg.AddDoubleField(AtmosphericPressure,PaToIHg,"%.2f","I" ) ) return;
if ( !NMEA0183Msg.AddDoubleField(AtmosphericPressure,PaToBar,"%.3f","B") ) return;
if ( !NMEA0183Msg.AddDoubleField(Temperature,KelvinToC,"%.1f","C" ) ) return;
// Following are ignored
if ( !NMEA0183Msg.AddDoubleField(NMEA0183DoubleNA,KelvinToC,"%.1f","C") ) return; //7 Water temperature,
...
sorry to be "thick" But would I be correct that PaToIHg and PaToBar are new and / or need to be defined?
as I cannot see them in the Library? Should they be "properly" in N2KMessages.h ?? Along the lines of :
inline double PaToBar(double v) { return N2kIsNA(v)?v:v/100000L; }
and
inline double PaToIHg(double v) { return N2kIsNA(v)?v:v/3386L; }
I did this in N2KMessages.h and then added your examples did not compile. After some exploring I think they should be :???
(you cannot use the PaToIHG conversion as a "multipler? -- and the Kelvinto C is a subtraction so not a multipler at all.
Or am I confused?
I tried what I think is the correct code, and this compiles with the "forced "1" in the multiplier input.
But I have not had a chance to see if it "works" as I do not have a version of the relevant transducers.
if ( !NMEA0183Msg.Init("MDA", "II") ) return;
if ( !NMEA0183Msg.AddDoubleField(PaToIHg(AtmosphericPressure),1,"%.2f","I" ) ) return;
if ( !NMEA0183Msg.AddDoubleField(PaToBar(AtmosphericPressure),1,"%.3f","B") ) return;
if ( !NMEA0183Msg.AddDoubleField(KelvinToC(Temperature),1,"%.1f","C" ) ) return;
??
No, they are just constant factor, which you define inside your own module.
static const double PaToIHg=0.00029529983071;
static const double PaToBar=0.00001;
There are similar definitions on NME0183Messages.cpp.
I do not prefer to modify library code, if you do not also update it to library. At this point it is not necessary.