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

Add functions to disable specific error types in SocketCANInterface #302

Closed

Conversation

redkite
Copy link

@redkite redkite commented Sep 26, 2018

Similar to #264, but so far limited to the SocketCANInterface only.

Saves the state of the CAN error mask within the object and provides two functions to disable lost arbitration and controller problems. These two have been added explicitly in order to not disable other errors by accident. A private function within SocketCANInterface still provides an internal, generic interface.

Could also be added to the socketcan bridge. I cannot test it though, because we are using the SocketCANInterface only.

Copy link
Member

@mathias-luedtke mathias-luedtke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patch!
I am afraid, I cannot merge it (as-is).
It introduces ABI breaks and adds new interfaces in a sub class.

As I have commented before, I don't think that changing the error mask is the right way to solve this.
The error should get white-listed here instead.

I could imagine a virtual, private function handleErrorFrame that could get configured or overridden in sub classes.

@redkite
Copy link
Author

redkite commented Oct 15, 2018

Sorry for the late reply, I was busy during the last week. I will have a look at possibilities to implement the changes and come back to you this week.

@redkite redkite closed this Oct 16, 2018
@redkite redkite force-pushed the ml/discard_arb_errors branch from 2872a0c to 2c09211 Compare October 16, 2018 13:34
@redkite redkite reopened this Oct 16, 2018
@redkite
Copy link
Author

redkite commented Oct 16, 2018

@ipa-mdl Is this more along the lines what you would expect? One could then inherit from SocketCANInterface and replace the error handler.

I am still not entirely sure, if the error mask should be hidden in the init function. If I would like to add CAN_ERR_BUSERROR as a handled error, I still would have to replace the complete init function. Could you imagine merging a PR whith a virtual function genErrorMask() to work around such issues, or do you deem this completely unnecessary?

@redkite redkite force-pushed the ml/discard_arb_errors branch 3 times, most recently from 56b7a70 to 3a28794 Compare October 16, 2018 14:11
@redkite redkite force-pushed the ml/discard_arb_errors branch from 3a28794 to adfe2f4 Compare October 16, 2018 14:50
@redkite
Copy link
Author

redkite commented Dec 4, 2018

@ipa-mdl Do you have any feedback on these changes?

@redkite
Copy link
Author

redkite commented Dec 20, 2019

Closing this, no PR response for a year.

@redkite redkite closed this Dec 20, 2019
@Timple
Copy link

Timple commented Dec 20, 2019

Still a valid PR. I think the discussion is meanly happening here: #264

But so far it the conversation seems to come to halt every time :(. Feel free to chip in there!

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

Successfully merging this pull request may close these issues.

3 participants