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

Support for berr-reporting #59

Closed
brian-brt opened this issue Nov 24, 2020 · 6 comments
Closed

Support for berr-reporting #59

brian-brt opened this issue Nov 24, 2020 · 6 comments

Comments

@brian-brt
Copy link
Contributor

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

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

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

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

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

brian-brt added a commit to BlueRiverTechnology/candleLight_fw that referenced this issue Dec 2, 2020
It reports more information with more accurate ordering with respect to
other frames sent and received.

Fixes candle-usb#59
This was referenced Dec 2, 2020
@fenugrec
Copy link
Collaborator

fenugrec commented Dec 2, 2020

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

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

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

Successfully merging a pull request may close this issue.

2 participants