Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is ExtendReceiveMessages necessary? #436

Open
DrStS opened this issue Nov 8, 2024 · 7 comments
Open

is ExtendReceiveMessages necessary? #436

DrStS opened this issue Nov 8, 2024 · 7 comments

Comments

@DrStS
Copy link

DrStS commented Nov 8, 2024

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
Copy link
Owner

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
Copy link
Author

DrStS commented Nov 8, 2024

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

@DrStS
Copy link
Author

DrStS commented Nov 12, 2024

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

@ttlappalainen
Copy link
Owner

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
Copy link
Author

DrStS commented Nov 13, 2024

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
Copy link
Author

DrStS commented Nov 13, 2024

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants