search

candle-usb / candleLight_fw

gs_usb compatible firmware for candleLight, cantact and canable
Other
642 stars 283 forks source link

Support for berr-reporting #59

Closed brian-brt closed 3 years ago

brian-brt commented 3 years ago

I've got a project with a CANable clone, and I'd like to get information about what kind of CAN errors it sees. SocketCAN supports this via the berr-reporting (aka CAN_CTRLMODE_BERR_REPORTING) feature. I'm planning to hook that up.

The CAN_ESR register on this hardware has a LEC field which exposes this information. I've worked with the c_can Linux driver before, which uses hardware with a very similar interface for bit errors, so I plan to follow that pattern. I'm planning to expose that information from this firmware, and add support for gs_usb to expose it via the existing SocketCAN APIs. It looks like the existing supported features / enabled features bitmasks should make this easy to implement in a fully backwards-compatible way on both sides.

I don't see a mailing list or anything, so I'm opening this issue to discuss. Is there another place for that? Does anybody here know of any similar or conflicting work? Any suggestions/comments on the order of sending out the changes here and on the kernel side?

fenugrec commented 3 years ago

I think this is, at the moment, the best place for this kind of discussion. It's a fairly quiet project; the creator @HubertD is busy on other things and I'm only trying to assist in my own limited time.

I currently don't have much t o say on this, but there is one "major" change coming "soon" (tm) - I'm supposed to merge the hang on suspend PR #51 (I really wanted to be able to reproduce the issue though), then the ST SPL update PR #27 which is already a complicated merge. So, fair warning - if you start implementing this and I merge 27 before you finish, you may have a slightly messy rebase to take care of : )

brian-brt commented 3 years ago

I went to look at the place to add this, and found that it's already there. Turns out candleLight currently reports bit errors unconditionally already... gs_usb doesn't allow turning the flag on though, which was the thing that confused me. I don't have a need to fix that. Just need to make sure the rest of my software isn't too clever about looking for that flag. Changing it in a backwards-compatible way would be a bit tricky too (old candleLight = always does it even when the flag is off, but new candleLight = won't report bit errors without flipping the flag?).

I do see some improvements to report more of the information exposed by the CAN peripheral though. Should have a PR for that ready soon.

fenugrec commented 3 years ago

ah nice. Error reporting is an area I'm not very familiar with. I'm all for improving the fw; I wonder how many applications would be "broken" by candle not being backwards-compatible ? And would the required "fix" be anything more than trivially enabling the proper mode when bringing the i/f up ?

brian-brt commented 3 years ago

That would be the fix. However, you'd need to upgrade the kernel side to know about the new flag first.

fenugrec commented 3 years ago

gs_usb doesn't allow turning the flag on though, which was the thing that confused me. I don't have a need to fix that.

finally got around to testing and I get what you mean now -

$ sudo ip link set can1 type can berr-reporting on
RTNETLINK answers: Operation not supported

How are you testing this ?

brian-brt commented 3 years ago

I'm running candump -ta can0,0:0,#FFFFFFFF to see the error frames, both before and after my change.