ttlappalainen / NMEA2000

NMEA2000 library for Arduino
542 stars 228 forks source link

is ExtendReceiveMessages necessary? #436

Open DrStS opened 2 weeks ago

DrStS commented 2 weeks ago

I have a rather basic question. What is the intended logic to receive a message. Based on the code examples I have thought the following:

Example: access altitude data from PGN 129029 in a multidevice setup:

const unsigned long TransmitMessagesTemperatureMonitor[] PROGMEM = {130310L, 130311L, 130312L, 130313L, 130314L, 130316L, 0};
const unsigned long ReceiveMessagesTemperatureMonitor[] PROGMEM = {129029L, 0};

in setup()

NMEA2000.SetMode(tNMEA2000::N2km_ListenAndNode, 169);
//NMEA2000.SetForwardStream(&Serial);
//NMEA2000.SetForwardType(tNMEA2000::fwdt_Text);

NMEA2000.ExtendTransmitMessages(TransmitMessagesBatteryMonitor, 0);
NMEA2000.ExtendTransmitMessages(TransmitMessagesTemperatureMonitor, 1);
NMEA2000.ExtendReceiveMessages(ReceiveMessagesTemperatureMonitor,1);
NMEA2000.SetMsgHandler(HandleNMEA2000Msg);
NMEA2000.SetOnOpen(OnN2kOpen);
NMEA2000.Open();

in loop() NMEA2000.ParseMessages(); I would expect now that in HandleNMEA2000Msg to see a PGN 129029

void HandleNMEA2000Msg(const tN2kMsg &N2kMsg)
{
    if (N2kMsg.PGN == 129029L)
    {
 // this code should be now executed
    }
}

I have checked further by uncommenting

//NMEA2000.SetForwardStream(&Serial);
//NMEA2000.SetForwardType(tNMEA2000::fwdt_Text);

that the PGN 129029 is recevied by the device. The SetForwardStream shows PGN 129029. However, in HandleNMEA2000Msg() I do not see any PGN 129029.

Is there an additional API call needed for a multi device receive application. Unfortunately, I have not seen such an example in the repo.

Thanks a lot! Best Stefan

ttlappalainen commented 2 weeks ago

ExtendTransmitMessages/Receive messages are just informative to the library. If other devices requests messages your device will handle, it returns list of message handled by library core and extended messages. It does not have any effect for handled messages.

I did not see anything wrong with your callback definition so HandleNMEA2000Msg should be called. You do not have any code in it. How do you know it has not been called? On NMEA2000.cpp line 2642 is call RunMessageHandlers after ForwardMessage. So HandleNMEA2000Msg will be called, if callback has been set.

DrStS commented 2 weeks ago

Thanks! I had a print statement in there. Where would be the best place to instrument the library?

DrStS commented 1 week ago

Thanks. It was an issue with freertos timing. Seems if the HandleNMEA2000Msg is run with less than 500Hz messages are dropped.

ttlappalainen commented 1 week ago

Which CAN controller you use?

CAN controller should receive messages to internal buffer by interrupt so that it is not critical to call tNMEA2000::ParseMessages all the time with 500 Hz. NMEA2000_Teensy, NMEA2000_Teensyx and NMEA2000_esp32 drivers has been proved to handle data properly. Also NMEA2000_mcp works, if it will be used in interrupt mode. There are some ESP32 driver versios, which I have not tested and know that they do not setup internal buffer properly. Check also tNMEA2000::SetN2kCANReceiveFrameBufSize

Even there is proper interrupt buffering, one should call tNMEA2000::ParseMessages as fast as possible so that message timing is good. Random delays should not be longer than 50 ms and average shoul be 1-2 ms.

DrStS commented 1 week ago

Thanks a lot! I have used a ESP32S3 and used this driver: https://github.com/sergei/NMEA2000_esp32_twai I have set tNMEA2000::SetN2kCANReceiveFrameBufSize already to a higher value --> 1024. Seems to have little impact. Best Stefan

DrStS commented 1 week ago

After changing the constructor call to: tNMEA2000 &NMEA2000 = *(new NMEA2000_esp32_twai(ESP32_CAN_TX_PIN, ESP32_CAN_RX_PIN, TWAI_MODE_NORMAL,128,128)); it seems to be resolved. Yet the NMEA2000 Api calls seem to have no effect for this driver.

ttlappalainen commented 1 week ago

NMEA2000_esp32_twai does not define buffers according tNMEA2000::SetN2kCANReceiveFrameBufSize so it does not work with library as designed. Anyway as you can setup buffers on contructor, it may work. I have not tested this driver with my tests or NMEA2000 certification tool, so I can not either say does it work properly.