Skip to content

Report errors#375

Merged
pavel-kirienko merged 6 commits intoOpenCyphal:masterfrom
tomasjakubik:report-errors
Feb 12, 2026
Merged

Report errors#375
pavel-kirienko merged 6 commits intoOpenCyphal:masterfrom
tomasjakubik:report-errors

Conversation

@tomasjakubik
Copy link
Contributor

Add hooks for errors

  • CAN controller errors via socketcan
  • when dropping a Tx frame

@tomasjakubik

This comment was marked as outdated.

@coveralls
Copy link

coveralls commented Jan 20, 2026

Coverage Status

coverage: 93.402% (-0.4%) from 93.785%
when pulling 463b291 on tomasjakubik:report-errors
into 9c97bd1 on OpenCyphal:master.

@tomasjakubik tomasjakubik force-pushed the report-errors branch 2 times, most recently from d53ad72 to d30391d Compare January 20, 2026 08:13
@tomasjakubik tomasjakubik marked this pull request as ready for review January 20, 2026 08:45
@tomasjakubik
Copy link
Contributor Author

tomasjakubik commented Jan 20, 2026

@pavel-kirienko Please have a look when you have time.
I am not sure that the hook/callback architecture is the way to go, so feel free to suggest something better.
I don't like the coverage decrease, but I am not sure if we can test this code reasonably without hardware 🤷.

Those two fixes for libpcap and SOL_CAN_RAW might probably go to a minor patch version if needed.
edit: Libcap was fixed and the commit dropped.

@tomasjakubik tomasjakubik force-pushed the report-errors branch 3 times, most recently from 9234e4d to e40f259 Compare January 26, 2026 07:31
@tomasjakubik
Copy link
Contributor Author

I think the missing setuptools came from the updated libpcap which changed the dependency.

As for the missing bpf_program it is probably also linked with changes in libpcap. I am looking into it.

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

This is fine but we need to avoid API breakage. Please tweak the arguments as suggested. You will also need to undo some of the changes to the test suite since they will no longer be necessary.

Comment on lines 95 to 96
error_handler: typing.Optional[ErrorHandler],
no_automatic_retransmission: bool,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
error_handler: typing.Optional[ErrorHandler],
no_automatic_retransmission: bool,
no_automatic_retransmission: bool,
error_handler: ErrorHandler | None = None,

A default is needed to avoid API breakage. Please update implementations.

@pavel-kirienko pavel-kirienko merged commit 58c356e into OpenCyphal:master Feb 12, 2026
9 checks passed
@tomasjakubik tomasjakubik deleted the report-errors branch February 12, 2026 15:34
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