ttlappalainen / NMEA2000

NMEA2000 library for Arduino
535 stars 226 forks source link

SetN2kTemperature, TempInstance, N2kts_ExhaustGasTemperature, issue? #137

Closed virtuvas closed 4 years ago

virtuvas commented 5 years ago

Timo,

setting up a new routine to show Exhaust Gas Temperature on a Maretron250 (since GMI10 doesn't support EGT...)

Maretron supports PGN130312 and not PGN130316, no problems here. First issue is that I use TempInstance in the way I'm using EngineInstance on PGN127489. I tried it changing from 0 to 1 and data moves from port to starboard engine, so it works. Am I OK, or missing something?

Now, the problem I'm facing (and which I don't know if it's Maretron or your lib issue) is that if I send a value HIGHER than 655K (approx 382C) to ActualTemperature values go crazy looping! I mean:

from 0 to 382C (using the CToKelvin() ) Maretron screen produces right results 383C gives -272 455C gives -200 and goes on like that... 655C gives 0 955C gives 300 1037C gives 382 again and 1038 gives again -273 and loops again increasing to go back -273 after another 655 Seems that the magic number is 655 after which system doesn't seem to accept anything more and restarts from zero.

On the other hand, if I send 500C to Transmission Oil Temp using PGN127493, everything is fine and Maretron reports correctly 500C.

Any idea what may be wrong?

cheers

V.

PS. below you can see the routine sending the PGN to the bus:

#define EngineParamsUpdatePeriod 1000

void EngineParams() {

  static unsigned long EngineParamsUpdated = millis();
  tN2kMsg N2kMsg;
  double AltVolt = 25.9;     //in volt
  double CoolantPress = 3500; //in mbar

  unsigned char SID = 0;
  unsigned char TempInst = 0;

  tN2kTempSource TempSource = N2kts_ExhaustGasTemperature;
  double EGT = 400;

  if ( EngineParamsUpdated + EngineParamsUpdatePeriod < millis() ) {
    SetN2kEngineDynamicParam(N2kMsg, EngineInst, N2kDoubleNA, N2kDoubleNA, N2kDoubleNA, AltVolt, N2kDoubleNA, N2kDoubleNA,  mBarToPascal(CoolantPress), N2kDoubleNA, N2kInt8NA, N2kInt8NA);
    NMEA2000.SendMsg(N2kMsg);
    SetN2kTemperature(N2kMsg, SID, TempInst, TempSource, CToKelvin(EGT), N2kDoubleNA);
    NMEA2000.SendMsg(N2kMsg);
    EngineParamsUpdated = millis();
  }
}
ttlappalainen commented 5 years ago

That is NMEA2000 fault and simple math. 130312 carries tempareture in two bytes unsigned. So max number is 65534 (65535 = NA ). This is with 0.01K resolution starting from 0 K. So max value is 655.34 K = 382.19 C. Maybe I should add to code check, which does not try to fill values outside of 0 K - 655.34 K. But what I should then use NA or 0 K/655.34 K.

I am not sure does 130312 actually support ECT. It uses in my library same enum type as 130316. ECT

PGN127493 instead has 2 bytes for temp, but 0.1 K resolution. So it can go up tp 6553.4 K.

Note that 130316 has temp in 3 bytes in 0.001 accuracy giving range -8388 K .. 8388 K. But I do not understand NMEA 2000 organization logic for <0 K!

I do not have official docs, so I do not know should Tempinstance on temp PGNs fix it to specific engine in case of engine relate source. You can have several cabin temperatures, but each must use different instance.

virtuvas commented 5 years ago

thanks Timo,

I was assuming some connection as the 655 did ring a bell, but hadn't noticed that they have two decimals on Kelvin for temperatures on EGT... I guess you cannot just edit and drop that to one decimal then? going up to 6553K is hot enough for anything on a boat I think :-) but will obviously break everything else, or not?

In my opinion the problem lies in that EGT is not included in the engine parameters dynamic (or even rapid!)

Regarding your wondering if indeed 130312 is meant to support EGT, I guess the answer is that if Maretron support EGT only on this PGN, it must be fine...

any other suggestions other than me manually editing the local file N2kMessages.cpp line 1293&1305 and removing a zero?

cheers

V.

virtuvas commented 5 years ago

Hm, changing the .cpp file doesn't work...

ttlappalainen commented 5 years ago

You can not make changes to N2kMessages.cpp how it fills up the data. The NMEA2000 format is fixed. If you change resolution from 0.01 to 0.1, then your 273 K will be shown as 27.3 K. Note that Maretron shows now right what you sent up to 282 C, so the library must send it in right format. This has been also tested with Actisense device, which is certified. I do not see any way to go above 282 C with 130312.

virtuvas commented 5 years ago

OK I see,

looking at Maretron documentation on TMP100 https://www.maretron.com/products/pdf/TMP100%20Datasheet.pdf

I see they mention: PGN130823 as a High Temp EGT specific sentence (googling about it seems to be a link with a hgv standard PGN130823/SPN2551 J1939-21? ) and then PGN128720 which is a proprietary Maretron EGT sentence.

do we have any info on how PGN130823 is formulated???

cheers

V,

ttlappalainen commented 5 years ago

No. They both are Maretron proprietary. You should have TMP100 and spy network to do reverse engineering with it.

virtuvas commented 5 years ago

So I'm completely stuck :(

Explains why Garmin doesn't support EGT on their devices. IMHO, that's rather poor for such a well thought standard :(

no other option then?

V.

ttlappalainen commented 5 years ago

I would not say that it is so well though and this is just one example. An other good example is manufacturer name. There is manufacturer number and every manufacturer keeps database on their devices. So if you have cetrified device from 2017 and there will be new manufacturer for Nk2 devices on 2019, their name will be shown on old devices just "Unknown manufacturer".

I expect you are doing this for yourself. Then it is not big deal to just send your EGT a divided by 10. So when your DSM250 shows 45, you know it it is 450. Maybe they update DSM250 some day to support 130316.

I have the same. There is no PGN to show how much I have chain out on anchor. So I show it as "Bait Well Temperature" 0 is sea level, 1.3 is up on position and e.g. -20 is 20 m down.

virtuvas commented 5 years ago

Timo,

yes doing it for myself, so no problem. *0.1 is what I'm going to do for the DSM, not a very big deal, just not "right" and since I'm spending hours and hours doing all that I'd rather have it working as it should... I'll also do flagCheckEngine to turn on the relevant light on the engine screen on the Garmin plotter and get the plotter to sound a warning when it happens. Third thing I'll do is add yet another custom PGN for EGT and show it on the f/b custom 5inch touch screen I'm setting up for the stabilisers. So I'm covered.

Regarding chain out, I may be able to help you there! Since you have GMI20, you can setup NMEA0183 sentences that the AutoAnchor black box was using and have that presented on the GMI with no issues. I've found the sentences and have it running on mine. Here is the relevant thread: http://www.ybw.com/forums/showthread.php?444238-anchor-chain-counter-to-NMEA0183-Garmin-proprietary-sentences-via-arduino long thread around post 34 is the relevant info. This is how it looks like on the GMI10 screen: http://fos.prd.uth.gr/vas/crafts/mitos/rebuilt6/NMEA_AA_2.jpg

The code for it is here http://fos.prd.uth.gr/vas/crafts/mitos/var/Autoanchor_MEGA_MCP.zip but it's 2yrs old and SD.h had some changes since and writing to the sdcard needs updating (appends instead of overwriting the data saved on the card...)

cheers

V.

ttlappalainen commented 5 years ago

Unfortunately I do not have NMEA0183 line to GMI20.

virtuvas commented 5 years ago

I think it's worth the 20euro or so for the cable :wink:

thanks, closing this!

V.

virtuvas commented 5 years ago

Timo,

reopening the thread, as I just had a look again and noticed that Maretron give some information on their proprietary EGT PGN130823 at the bottom of their manual (https://www.maretron.com/support/manuals/TMP100UM_1.2.html). Tried to formulate a custom PGN but failed (no surprise there...) could you possibly help a bit so that I can try it on my setup?

PGN 130823 – Temperature, High Range (Maretron Proprietary)

The TMP100 uses this PGN to provide a regular transmission of temperature. The factory default for periodic transmission rate is once every two seconds. The transmission of this PGN can be disabled (see PGN 126208 – NMEA Request Group Function – Transmission Periodic Rate).

Field 1:   Manufacturer Code – This field contains Maretron’s NMEA 2000® manufacturer code, which is 137.
           2:    Reserved – The TMP100 sets all bits in this field to a value of “1”.
           3:    Industry Group – This field contains the Marine industry group code, which is 4.
           4:    SID – The sequence identifier field is used to tie related PGNs together (i.e., the data from each PGN was taken at the same time although they are reported at slightly different times).
           5:    Temperature Instance – The TMP100 sets this field to identify a particular temperature measurement from the source specified in Field 6.  Every temperature measurement from a given source type on the network should have a distinct instance value, so that monitoring devices and displays can identify which measurement is which.
           6:    Temperature Source – This field is used to indicate the type of temperature measurement being taken. Possible values for this field include Sea Temperature, Outside Temperature, Inside Temperature, Engine Room Temperature, Main Cabin Temperature, Live Well Temperature, Bait Well Temperature, Refrigeration Temperature, Heating System Temperature, Dew Point Temperature, Apparent Wind Chill Temperature, Theoretical Wind Chill Temperature, Heat Index Temperature, Freezer Temperature, Exhaust Gas Temperature, and User Defined #129-#144.
           7:    Actual Temperature – This field is used to indicate the temperature, whose source is specified in fields 5 and 6, in units of 0.1°C.
           8:    Set Temperature – The TMP100 sets this field to a value indicating “data not available”.

based on that and copying/modifying the PGN130316, I did the following:

on the N2kMessages.cpp:

uint16_t MARETRONMfgCode=137; 

//*****************************************************************************
// Temperature MARETRON TMP100
// Temperatures should be in Celcius
void SetN2kPGN130823(tN2kMsg &N2kMsg, unsigned char SID, unsigned char TempInstance, tN2kTempSource TempSource,
                     double ActualTemperature, double SetTemperature) {
    N2kMsg.SetPGN(130823L);
    N2kMsg.Priority=5;
    MARETRONMfgCode&=0x7ff; // Just for sure filter over 11 bits away.
    MARETRONMfgCode|=0x9800; // Add industry group =4 and 2 reserved bits [and the 11bits after it...].
    N2kMsg.AddByte(SID);
    N2kMsg.AddByte((unsigned char)TempInstance);
    N2kMsg.AddByte((unsigned char)TempSource);
    N2kMsg.Add2ByteDouble(ActualTemperature,0.1);
    N2kMsg.Add2ByteDouble(SetTemperature,0.1);
    N2kMsg.AddByte(0xFF); // Reserved.
}

bool ParseN2kPGN130823(const tN2kMsg &N2kMsg, unsigned char &SID, unsigned char &TempInstance, tN2kTempSource &TempSource,
                     double &ActualTemperature, double &SetTemperature) {
  if (N2kMsg.PGN!=130823L) return false;
  int Index=0;
  uint16_t MARETRONMfgCode;

  MARETRONMfgCode=N2kMsg.Get2ByteInt(Index); 
  if ( (MARETRONMfgCode & 0x7ff) != MARETRONMfgCode ) return false; // Not from Vosper device
  if ( (MARETRONMfgCode & 0xe000) != 0x8000 ) return false; // Check industry code is 4
  SID=N2kMsg.GetByte(Index);
  TempInstance=N2kMsg.GetByte(Index);
  TempSource=(tN2kTempSource)(N2kMsg.GetByte(Index));
  ActualTemperature=N2kMsg.Get2ByteDouble(0.1,Index);
  SetTemperature=N2kMsg.Get2ByteDouble(0.1,Index);

  return true;
}

and on N2kMessages.h:

//*****************************************************************************
// Temperature MARETRON TMP100
// Temperatures should be in Kelvins??? or in 0.1C as per Maretron doc
// Input:
//  - SID                   Sequence ID.
//  - TempInstance          This should be unique at least on one device. May be best to have it unique over all devices sending this PGN.
//  - TempSource            see tN2kTempSource
//  - ActualTemperature     Temperature in °K. Use function CToKelvin, if you want to use °C.
//  - SetTemperature        Set temperature in °K. Use function CToKelvin, if you want to use °C. This is meaningfull for temperatures,
//                          which can be controlled like cabin, freezer, refridgeration temperature. God can use value for this for
//                          outside and sea temperature values.
// Output:
//  - N2kMsg                NMEA2000 message ready to be send.
void SetN2kPGN130823(tN2kMsg &N2kMsg, unsigned char SID, unsigned char TempInstance, tN2kTempSource TempSource,
                     double ActualTemperature, double SetTemperature=N2kDoubleNA);

inline void SetN2kMaretronTMP100(tN2kMsg &N2kMsg, unsigned char SID, unsigned char TempInstance, tN2kTempSource TempSource,
                     double ActualTemperature, double SetTemperature=N2kDoubleNA) {
  SetN2kPGN130823(N2kMsg,SID,TempInstance,TempSource,ActualTemperature,SetTemperature);
}

bool ParseN2kPGN130823(const tN2kMsg &N2kMsg, unsigned char &SID, unsigned char &TempInstance, tN2kTempSource &TempSource,
                     double &ActualTemperature, double &SetTemperature);
inline bool ParseN2kMaretronTMP100(const tN2kMsg &N2kMsg, unsigned char &SID, unsigned char &TempInstance, tN2kTempSource &TempSource,
                     double &ActualTemperature, double &SetTemperature) {
  return ParseN2kPGN130823(N2kMsg, SID, TempInstance, TempSource, ActualTemperature, SetTemperature);
}

I recon the .h is okay, and the .cpp is wrong around the MARETRONMfgCode lines, could you please help? We don't know how long the PGN is, we don't know if it's a 2ByteDouble or a 3ByteDouble for the ActualTemperature, and I've just copied over the lines from the custom PGNs you helped me built last winter but not sure what I'm doing there.

cheers

V.

ttlappalainen commented 5 years ago

Some notes.

And parser would be

bool ParseN2kPGN130823(... if (N2kMsg.PGN!=130823L) return false; int Index=0; if ( N2kMsg.Get2ByteInt(Index)!=MaretronProprietary ) return false; ...

- You have to test length of actual temperature. I would guess it is 3 byte as on 130316, since otherwice they could just use 130316, but it did not existes, when they made TP100

- Note also that you can not read this message from bus without telling to library that it is fastpacket. So you must do in your initialization:

... const unsigned long ExtendedFastPacketMessage[] PROGMEM={130823UL,0}; ... void setup() { ... NMEA2000.ExtendTransmitMessages(ExtendedFastPacketMessage); ... NMEA2000.Open(); ... } ...

virtuvas commented 5 years ago

thanks again Timo, progressing but painfully slow!

yes, my plan was to put this in another library like I did with my other custom PGNs (only they were all single-frame). So, now my N2kMaretron.cpp is:

#include "N2kMessages.h"
#include "N2kMaretron.h"
#include <string.h>

#define MaretronProprietary 0x9889 // Maretron 137 + reserved + industry code=marine

//*****************************************************************************
// Temperature MARETRON TMP100
// Temperatures should be in Celcius
void SetN2kPGN130823(tN2kMsg &N2kMsg, unsigned char SID, unsigned char TempInstance, tN2kTempSource TempSource,
                     double ActualTemperature, double SetTemperature) {
    N2kMsg.SetPGN(130823L);
    N2kMsg.Priority=5;
    N2kMsg.Add2ByteUInt(MaretronProprietary);
    N2kMsg.AddByte(SID);
    N2kMsg.AddByte((unsigned char)TempInstance);
    N2kMsg.AddByte((unsigned char)TempSource);
    N2kMsg.Add3ByteDouble(ActualTemperature,0.1);
    N2kMsg.Add2ByteDouble(SetTemperature,0.1);
    //N2kMsg.AddByte(0xFF); // Reserved.
}

bool ParseN2kPGN130823(const tN2kMsg &N2kMsg, unsigned char &SID, unsigned char &TempInstance, tN2kTempSource &TempSource,
                     double &ActualTemperature, double &SetTemperature) {
  if (N2kMsg.PGN!=130823L) return false;
  int Index=0;
  if (N2kMsg.Get2ByteInt(Index)!=MaretronProprietary ) return false;
  SID=N2kMsg.GetByte(Index);
  TempInstance=N2kMsg.GetByte(Index);
  TempSource=(tN2kTempSource)(N2kMsg.GetByte(Index));
  ActualTemperature=N2kMsg.Get3ByteDouble(0.1,Index);
  SetTemperature=N2kMsg.Get2ByteDouble(0.1,Index);
  return true;
}

.h is not changed.

I did the suggested:

const unsigned long ExtendedFastPacketMessage[] PROGMEM={130823UL,0}; Q: does it really need the suffix UL??? all my other sentences have L as suffix!

and in the setup I added:

NMEA2000.ExtendTransmitMessages(ExtendedFastPacketMessage); but I also tried:

NMEA2000.ExtendFastPacketMessages(ExtendedFastPacketMessage); as I think that's the right one, ExtendTransmitMessages is used for all my "normal" messages, right?

anyway, either way, I have the problem that NMEA Reader reports the PGN130823 as being:

Proprietary Fast packet (obviously from the PGN# which is in this group of PGNs

Reports the right priority, BUT Length-8 whereas my packet length is 10 (or more...)

As a result, the values reported are wrong: NMEA Manufacturer code is looping through various values Industry code is always 0 and not 4 and interval is half of what I've declared in the code...

So, I must be missing something that will force the system understand that there are two packets in that PGN.

BTW, I even tried adding: N2kMsg.DataLen=10; but didn't work.

N2kMsg.DataLen=11; gives a Industry code of 3, Manufacturer code is again looping all over

N2kMsg.DataLen=12; gives a Industry code of 7, Manufacturer code is again looping all over

cheers

V.

ttlappalainen commented 5 years ago

The UL suffix is right, since PGNs has been defined as unsigned long. L works since none of the values set to PGN does not exceed 2147483648. It is my mistake and should have used UL from the beginning.

ExtendTransmitMessages just informs library to tell other systems that device will output that message. It is critical for fullfill NMEA2000 specifications, but will work fine without.

ExtendFastPacketMessages is for telling library that some message is fast packet. Unfortunately NMEA2000 does not have any logic to know is message single frame or fast packet, so library must just have list of fast packet messages. Library will send messages longer that 8 bytes automatically as fp, but it will not receive them right, if it does not know that it should collect coming frames to one message.

So if you read and handle that message you must have ExtendFastPacketMessages and should have also ExtendReceiveMessages on your setup. If you send that message you should have ExtendTransmitMessages on your setup. In sending case since message is longer that 8, it will anyway send it as fast packet. So if you only send that message you do not need ExtendFastPacketMessages, but it either does not harm much (just some extra testing) to have it.

You should not touch N2kMsg.DataLen. It will be set according to N2kMsg.Addxxx, when you build the message. That was also my mistake to leave those public, while they should be protected. I'll may fix that some day.

Now the problem may be with NMEA Reader. Even it reports message proprietary fast packet it may be that it does not handle it as fast packet. In that case it would handle each frame as single message and every time read again mfg id, which is different on second frame since it contains set temperature data. You can check that library really send it as fast packet by adding some Serial.println(...) message on NMEA200.cpp above line 1057 unsigned char temp[8]; // {0,0,0,0,0,0,0,0};, which is elese side of decision can it be handled single frame. I'll bet that your message will be sent right and NMEA Reader handles it wrong.

Have you tested it with Maretron display?

virtuvas commented 5 years ago

works!

I don't have enough drop cables at home so I wanted to see it working properly in NMEA Reader before plugging the Maretron. Sure, plugged it in and it works! Well done Timo!

Final note, in my includes at the top I have:

// NMEA2000 includes

// List here messages your device will transmit.
// Engine Rapid 127488
// Engine Dyn   127489 [Fuel flow lt/h works on Garmin]
// Transmission 127493
// Trip params  127497 [fuel]
// Temp PGN     130311,130312 [130311 to be phased out, 130316 not supported for EGT]
// EGT Maretron custom Maretron PGN 130823
// TrimTab PGN  130576
// my custom PGN 65522 
const unsigned long TransmitMessages[] PROGMEM ={127488L,127489L,127493L,130312L,130576L,130306L,130823L,65522L,0};

const unsigned long ExtendedFastPacketMessage[] PROGMEM={130823L,0};

and in my setup the following:

   NMEA2000.SetProductInformation("1212136",                     // Manufacturer's Model serial code
                                 1210,                           // Manufacturer's product code
                                 "GVD ECU-STBRD",                // Manufacturer's Model ID
                                 SWVER,
                                 "GVD N2K ENG_V1.4",             // Manufacturer's Model version
                                 0x02,                           // load equivalency - use default
                                 0xffff,                         // NMEA 2000 version - use default
                                 0xff                            // Certification level - use default
                                );
   NMEA2000.SetDeviceInformation(1212136,  // Unique number. Use e.g. Serial number.
                                160,       // Engine Gateway     // Device function. 
                                50,        // Propulsion         // Device class. 
                                1210,      // Just choosen free from code list
                                4          // Marine
                               );

  NMEA2000.SetMode(tNMEA2000::N2km_NodeOnly, 136);
  NMEA2000.EnableForward(false); // Disable all msg forwarding to USB (=Serial)
  // Here we tell, which PGNs we transmit for each device
  NMEA2000.ExtendTransmitMessages(TransmitMessages);
  NMEA2000.ExtendFastPacketMessages(ExtendedFastPacketMessage);
  NMEA2000.Open();

everything works even if I comment out the last three lines...

I'll probably have another go at decrypting a couple more Maretron sentences. When I finish, how shall I sent them to you for including to your work? I'm only using github to get things, no idea how you sent files via it.

cheers

V.

ttlappalainen commented 5 years ago

Good!

As I explained, you do not need ExtendFastPacketMessages, if you do not read that message with library. ExtendTransmitMessages if informative, if some other device requests messages you support. It works without, but just keep it.

Open will be done automatically on ParseMessages, if you have not done it.

For github easiest is to have GitHub desktop. That has some hacker logic and requires some studying. If you do not do much returning stuff, I can add those files for you. Just copy copyright from my code and change author and year. I hope MIT lisence is OK for you. GPL causes trouble and should not be used.

virtuvas commented 5 years ago

OK, will study a bit, you mean I'll have to create a Repository and then add my two files there and somehow link to your Repositories, is that the general idea?

BTW, I managed to almost get PGN65286 (Maretron proprietary Fluid Flow Rate) working using the code below but I have an odd issue, maybe you could help from your experience:

// Fluid Flow Rate [MARETRON FFM100]
/*
Field      1:   Manufacturer Code – This field contains Maretron’s NMEA 2000® manufacturer code, which is 137.
           2:    Reserved – The FFM100 sets all bits in this field to a value of “1”.
           3:    Industry Group – This field contains the Marine industry group code, which is 4.
           4:    SID – The sequence identifier field is used to tie related PGNs together.
           5:    Flow Rate Instance – The FFM100 sets this field to identify a particular fluid flow measurement from the fluid type specified in Field 6.  Every flow measurement from a given fluid type on the network should have a distinct instance value, so that monitoring devices and displays can identify which measurement is which.
           6:    Fluid Type – This field is used to indicate the type of fluid flow measurement being taken. Possible values for this field include Fuel, Fresh Water, Waste Water, Live Well, Oil, and Black Water.
           7:    Reserved bits – The FFM100 sets all bits in this field to a value of “1”.
           8:    Fluid Flow Rate – This field is used to indicate the rate of fluid flow in units of 1x10-4 m3/hour.
*/
// Flow unit is 0.1 lt/hour
void SetN2kPGN65286(tN2kMsg &N2kMsg, unsigned char SID, unsigned char FlowRateInstance, tN2kFluidType FluidType,
                     double FluidFlowRate) {
    N2kMsg.SetPGN(65286L);
    N2kMsg.Priority=5;
    N2kMsg.Add2ByteUInt(MaretronProprietary);
    N2kMsg.AddByte(SID);
    N2kMsg.AddByte((unsigned char)FlowRateInstance);
    N2kMsg.AddByte((unsigned char)FluidType);
    N2kMsg.AddByte(0xFF); // Reserved.
    N2kMsg.Add2ByteDouble(FluidFlowRate,1);
}

Problem is that values are not what I'm sending. I think it's an issue of picking garbage and using it, but I'm running out of ideas. Maybe it ticks on you:

1l/h and the Maretron shows 51 10l/h -> 282 100l/h -> 2586 1000l/h -> 25626 10000l/h -> 256026

so 0.01l/h -> 25.5 0.1l/h -> 25.5 (???) 5l/h -> 154 50l/h ->1306 500l/h -> 12826

if I change (FluidFlowRate, 0.1) things shift up

makes sense? tried adding two x 0xff before the data, I get nothing tried Add3ByteDouble, nothing tried all sorts of combinations of adding 1-3 xFF to adding two-three ByteDouble, didn't work.

I thought it would be similar to the other proprietary page I setup so there's going to be 3byteDouble and a 2ByteDouble, one with data the other filled with FF. Nope.

ideas welcomed!

cheers

V.

ttlappalainen commented 5 years ago

You did not mention does Maretron show instance and fluid type right. This tells are first fields define right. See e.g. PGN127505, where instance and fluid type has been packed to one byte.

PGN65286 is proprietary single frame message, so its length is max. 8 bytes.

Also your units are a bit confusing. For function call do not use crazy unit like 0.1 l/h. It should be SI unit cubic meter/s. More common l/h is also OK, since also NMEA 2000 uses l/h.

Anyway you comment that function unit is 0.1 l/h, and then you say e.g. 10l/h -> 282 That confused a bit, since you actually fill to the code with coefficient 1. So you did not feed 100 to function. When I realized these I found that your data is shifted. So the right code is (I think): // Flow unit is litres/hour void SetN2kPGN65286(tN2kMsg &N2kMsg, unsigned char SID, unsigned char FlowRateInstance, tN2kFluidType FluidType, double FluidFlowRate) { N2kMsg.SetPGN(65286L); N2kMsg.Priority=5; N2kMsg.Add2ByteUInt(MaretronProprietary); N2kMsg.AddByte(SID); N2kMsg.AddByte((FlowRateInstance&0x0f) | ((FluidType&0x0f)<<4)); N2kMsg.AddByte(0xFF); // Reserved. N2kMsg.Add2ByteDouble(FluidFlowRate,0.1); N2kMsg.AddByte(0xFF); // Reserved. }

ttlappalainen commented 5 years ago

Fix: You seem to get high rates like 256026, which is not possible with 2 byte. So code must be: // Flow unit is litres/hour

void SetN2kPGN65286(tN2kMsg &N2kMsg, unsigned char SID, unsigned char FlowRateInstance, tN2kFluidType FluidType,
double FluidFlowRate) {
N2kMsg.SetPGN(65286L);
N2kMsg.Priority=5;
N2kMsg.Add2ByteUInt(MaretronProprietary);
N2kMsg.AddByte(SID);
N2kMsg.AddByte((FlowRateInstance&0x0f) | ((FluidType&0x0f)<<4));
N2kMsg.AddByte(0xFF); // Reserved.
N2kMsg.Add3ByteDouble(FluidFlowRate,0.1);
}
virtuvas commented 5 years ago

thanks for the ideas Timo, but something is still wrong...

clarifications the two lines below are right:

    N2kMsg.AddByte((unsigned char)FlowRateInstance);
    N2kMsg.AddByte((unsigned char)FluidType);

I mean I can change engine and change fluid type on my arduino code and Maretron sees it, So I think we have to stick to these two lines. Furthermore, replacing them with the suggested:

N2kMsg.AddByte((FlowRateInstance&0x0f) | ((FluidType&0x0f)<<4)); doesn't work. I mean I get no values at all on the Maretron display. Now this is consistant with the one I sent you in terms of frame size and number of bytes (8) as we have 5 bytes up to there, 1byte reserved and 2BytesDouble for the value. Frankly that's the only one that has produced results in the Maretron screen so far (and I spend the best part of last evening playing with the setup and values...)

The table of values reported by Maretron is with:

N2kMsg.Add2ByteDouble(FluidFlowRate,1);

so, ltrs/h in arduino code on the left, Maretron display on the right: 1l/h -> 51.1 10l/h -> 282 100l/h -> 2586 1000l/h -> 25626 10000l/h -> 256026 100000 l/h -> -795418 1mil -> 434202 11mil -> -691379 I see no pattern on the values tbh

below is the code I'm using to sent the data to the bus. Note that as in the other maretron pgn, I equal SID with FlowRateInst. Well, it works fine, just the values are wrong!

#define TripParamsUpdatePeriod 500

void TripParams() {
  tN2kMsg N2kMsg;
  unsigned char FlowRateInst = EngineInst;
  tN2kFluidType FluidType = N2kft_Fuel; 

  double FuelFlow = 100000;  // in lt per hour

  if ( TripParamsNextUpdate < millis() ) {
    TripParamsNextUpdate  += TripParamsUpdatePeriod;
    SetN2kMaretronFluidFR(N2kMsg, EngineInst, FlowRateInst, FluidType, FuelFlow);
    NMEA2000.SendMsg(N2kMsg);
  }
}

your views please!

V.

ttlappalainen commented 5 years ago

Try then:

void SetN2kPGN65286(tN2kMsg &N2kMsg, unsigned char SID, unsigned char FlowRateInstance, tN2kFluidType FluidType, double FluidFlowRate) {
  N2kMsg.SetPGN(65286L);
  N2kMsg.Priority=5;
  N2kMsg.Add2ByteUInt(MaretronProprietary);
  N2kMsg.AddByte(SID);
  N2kMsg.AddByte((unsigned char)FlowRateInstance);
  N2kMsg.AddByte((unsigned char)FluidType || 0xf0 );
  N2kMsg.Add3ByteDouble(FluidFlowRate,0.1);
}
ttlappalainen commented 5 years ago

The patter is that your data is shifted, if you have one full reserved byte between value and fluid type. So you set 1 -> 0000 0001b. In your data bytes are at end 1111 1111 0000 0001 0000 0000, first ff is first byte and we get number 0000 0000 0000 0001 1111 1111b = 511 0.1 = 51.1 l/h In similar: 100l/h -> 2586 1111 1111 0110 0100 0000 0000 and as number 0000 0000 0110 0100 1111 1111b = 25855 0.1 = 2586 l You can calculate rest if you like.

virtuvas commented 5 years ago

👍

void SetN2kPGN65286(tN2kMsg &N2kMsg, unsigned char SID, unsigned char FlowRateInstance, tN2kFluidType FluidType,
                     double FluidFlowRate) {
    N2kMsg.SetPGN(65286L);
    N2kMsg.Priority=5;
    N2kMsg.Add2ByteUInt(MaretronProprietary);
    N2kMsg.AddByte(SID);
    N2kMsg.AddByte((unsigned char)FlowRateInstance);
    N2kMsg.AddByte((unsigned char)FluidType);
    N2kMsg.Add3ByteDouble(FluidFlowRate,0.1);
}

works! note without your || 0xf0 on FluidType.

so seems that Maretron changed their mind from the description in their own manual to the actual product? Oh, well, the important thing is that it works! will sort out the code and see if I can do it or sent it to you to do it.

thanks a lot!

V.

ttlappalainen commented 5 years ago

Doesn't it work with || 0xf0? Note that reserved field does not mean full byte field. There are lot of examples, where data is only 2-4 bit and rest of byte is reserved. Somewhere is also so that data is 4 bit and then there is 12 bit reserved.

If it does not work with || 0xf0, then we have to live with that and forget reserved field.

ttlappalainen commented 5 years ago

Is there period mentioned on Maretron document?

virtuvas commented 5 years ago

no, it doesn't work with || 0xf0 at all, so we just forget the reserved field.

Manual states that "The factory default for periodic transmission rate is twice every second" I've amended my arduino code above to 500 millis.

So, looks like this is done as well!

V.

ttlappalainen commented 5 years ago

Good. So if you accept MIT, I'll send CANBOAT guys information of those PGN data structure.

virtuvas commented 5 years ago

yep MIT is absolutely fine.

just attaching the files [added my name last in the Copyright since I'm just playing/testing and you've done the real work!]

N2kMaretron.zip

cheers

Vassilis

virtuvas commented 5 years ago

will try PGN 65287 –Trip Volume in the afternoon, structure should be identical to the 65286, just cumulative of lts used on the trip (if I got that right) useful for motorboaters, not so for sailors...

ttlappalainen commented 5 years ago

Some comments.

ttlappalainen commented 5 years ago

Also wasn't resolution on actual temperature 0.001?

ttlappalainen commented 5 years ago

Also on 130823 is Set temperature signed or unsigned? I would expect it is unsigned and then you should use N2kMsg.Add2ByteUDouble. You can test it by checking from Maretron what it shows for NA. If signing is right, it either does not show it or shows "Not available". If it is wrong, it shows somw crazy value.

virtuvas commented 5 years ago

Timo, my replies below:

again I started by copying your 130316, it worked I left it like that. I'll look at it in the evening I have to go to the boat and do some work and testing right now before the weather turns foul. However, does it really make any difference, can you actually use SetTemperature??? how??? I mean for EGT in particular, it would mean that you set a value (HOW???) and then when it's reached the electronic governor of the engine cuts fuel and drops revs?? Sorry I'm probably missing the whole concept of SETting values from a screen (I guess) via NMEA to an actual subsystem. can you elaborate?

I'll sent you an updated version with the PGN65287 included in the evening.

cheers

V.

ttlappalainen commented 5 years ago

You have to test and confirm coefficient for 130823 Actual temperature. Maretron document says: 7: Actual Temperature – This field is used to indicate the temperature, whose source is specified in fields 5 and 6, in units of 0.1°C. , but why they would have resolution only 0.1 since then they would get temperatures up to 838800 C. With 0.001 as in 130316 it goes up to 8388 C, which should be enough on boats.

The only way is to test with code what Maretron shows and find right coefficient.

ttlappalainen commented 5 years ago

Actually got idea that you have not tested is Actual Temperature actually 2 byte: N2kMsg.Add2ByteUDouble(ActualTemperature,0.1); N2kMsg.Add2ByteUDouble(SetTemperature,0.1);

as in PGN 130312

For testing there are some ways:

Also could you check that values are in K. So if you set with function CToKelvin(0), does it show 0 °C on Maretron.

virtuvas commented 5 years ago

Timo,

did some further testing as suggested in your last post:

N2kMsg.Add3ByteDouble(ActualTemperature,0.1);
N2kMsg.Add2ByteDouble(SetTemperature,0.1);

N2kMsg.Add2ByteDouble(ActualTemperature,0.1);
N2kMsg.Add2ByteDouble(SetTemperature,0.1);

N2kMsg.Add2ByteUDouble(ActualTemperature,0.1);
N2kMsg.Add2ByteUDouble(SetTemperature,0.1);

all three options give identical results:

CToKelvin(N2kDoubleNA) gives -273 on the Maretron display CToKelvin(-273) gives -273 CToKelvin(0) gives 0 CToKelvin(6280) gives 6280 CToKelvin(6281) gives -273 CToKelvin(6282) gives -272 CToKelvin(50000) gives 4125 anyway, you get the idea...

display doesn't show decimals and to be honest, there's no point showing decimals on the values I'm measuring (EGT)

so, what do we use???

V.

ttlappalainen commented 5 years ago

Since 50000 and 6281 does not give right result and value runs over, it is clearly 2 byte unsigned. So

N2kMsg.Add2ByteUDouble(ActualTemperature,0.1);
N2kMsg.Add2ByteUDouble(SetTemperature,0.1);

Should be right.

virtuvas commented 5 years ago

added the Trip volume as well and corrected all the bits we discussed.

N2kMaretron_v2.zip

ttlappalainen commented 5 years ago

I updated the library now. Please test your code with new library. There were some minor changes to N2kMaretron module.

I also added proprietary fast packet test as default. So if you use ActisenseListener example for reading data, rebuild it and it will show 130823 on NMEA Reader right.

virtuvas commented 5 years ago

oops, sorry forgot this thread! yep, checked everything, updated lib works fine, so closing this thread.

cheers

V.

virtuvas commented 4 years ago

Timo,

just tried parsing the PGN130823 for my project, and realised there's a tiny error that slipped in there:

N2kMaretron.cpp:

line 54, 81 & 107:

if (N2kMsg.Get2ByteInt(Index)!=MaretronProprietary ) return false; should be: if (N2kMsg.Get2ByteUInt(Index)!=MaretronProprietary ) return false;

cheers

V.