sebi2k1 / node-can

NodeJS SocketCAN extension
223 stars 73 forks source link

Error Message Filter #90

Closed bdcdekimo closed 7 months ago

bdcdekimo commented 3 years ago

I would like to filter out certain error messages coming from the CAN socket. I see all errors are enabled in the RawChannel constructor:

      err_mask = CAN_ERR_MASK;
      if (setsockopt(m_SocketFd, SOL_CAN_RAW, CAN_RAW_ERR_FILTER, &err_mask, sizeof(err_mask)) != 0)

but as far as I can see, there is no way for the user to adjust the filter?

juleq commented 3 years ago

Hi. Off the top of my head: You can just test for the presence of msg.err in your receive handler and skip processing this msg if you want to. All in js code. Was that your goal?

bdcdekimo commented 3 years ago

Hi, that's what we are doing at the moment. The problem is that we receive an error message (with CAN_ERR_CRTL_TX_PASSIVE set) about once per ms when no other devices are present on the CAN bus. We would like to filter out those messages because they slow down our Node.js application.

juleq commented 3 years ago

I had a similar issue with the whole node process blocking on tx queue full. See #77. @sebi2k1 fixed that for me by introducing the non blocking option.

So when sending, you can check for tx queue full in advance and stop sending if full.

juleq commented 3 years ago

To clarify: Maybe the slowdown is not due to the 1ms error interval, but due to blocking send on tx queue full in between. One test every 1ms should not be a big performance hit... Just try the non_block_send option on channel construction.

bdcdekimo commented 3 years ago

Thanks, that has improved our performance quite a bit. I'm testing our performance in a pretty rudimentary way: I'm simply doing 10000 pretty simple REST API calls from my pc to the device. (These don't have any effect on what happens on the CAN bus). It takes about 20% less time to perform these REST API calls now that I'm using non-blocking sends on socketcan. So discounting network round trip times etc, that's actually a big performance improvement!

But unfortunately that is still about twice as long as it takes to perform the same calls when there is another CAN bus device present and there are no CAN bus errors. That 1 error per 1ms was an underestimation from my end: it's at least 10 errors per ms and that's probably the maximum because one of our CPU cores is used practically completely now (CPU usage drops below 1% if there are no CAN bus errors).

juleq commented 3 years ago

I guess that the behavior for the creation of these events is specific to the driver that implements the linux socketcan API. I recall having two targets that did not reveal my issue and one that did. I did not check if there are socketopts to control said behavior.

Did you see that you can check for a full tx queue by testing the send return value for -1? When you stop sending then you might get rid of all the events.

An alternative approach would be to latch, throttle or filter error events in the native part of this module. But that would need implementation :).

bdcdekimo commented 3 years ago

I didn't know about the return value. I'll take a look at it. Our application is only sending one message per second though. I can probably throttle sending a little bit, but not that much, because I'll need to send periodically to detect that the bus is "active" again. So to be honest I doubt this will stop the driver from spamming errors.

My suggestion would be to add a function setErrorFilters similar to setRxFilters to allow the user to set CAN_RAW_ERR_FILTER (similar to CAN_RAW_FILTER). Alternatively you could make the error mask an optional field in createRawChannelWithOptions.

bdcdekimo commented 3 years ago

I did some more tests: send indeed returns -1. Unfortunately just one transmission failure is enough to trigger a continuous flood of error events. The only way to make the events stop, is stopping the rawChannel. Then we have to create a new channel, start it and cross our fingers that there is another device present on the CAN bus this time...

juleq commented 3 years ago

Have you tried your masking idea? You can checkout the package, edit just the cxx file to test it, build it using npm and then give that a try.

bdcdekimo commented 3 years ago

Masking the errors in the RawChannel constructor fixed our problem. Could you make the error mask configurable in the library? If not we'll apply our own patch to the library in our build process.

sebi2k1 commented 3 years ago

I am happy to merge a pull request that feature or you may wanna share that patch and I can look into adopting it.

Do you just want to get rid of error messages in total or have it actually filtered for certain error messages?

bdcdekimo commented 3 years ago

I finally got around to making the modification. I've added a new method setErrorFilters to RawChannel that allows you to filter for all or only certain messages. See the patch in attachment. add-setErrorFilters.patch.txt

bdcdekimo commented 3 years ago

Hi, have you had a chance yet to take a look at the patch? Ideally we would like to simply use a new version of socketcan in our build system (bitbake) instead of looking for ways to apply a patch in our builds.

sebi2k1 commented 3 years ago

i am on it

sebi2k1 commented 7 months ago

Missed to close it with commit 2e5a7ab