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

Ixxat bus state and hardware errors detection #1141

Merged
merged 27 commits into from
Nov 14, 2022
Merged

Conversation

cowo78
Copy link
Collaborator

@cowo78 cowo78 commented Oct 18, 2021

Implemented IXXATBus.state() property.
Hardware errors are detected in _recv_internal() and raise an exception.

@mergify mergify bot requested a review from hardbyte October 18, 2021 08:44
@codecov
Copy link

codecov bot commented Oct 18, 2021

Codecov Report

Merging #1141 (f92a056) into develop (8c4ed5f) will decrease coverage by 0.07%.
The diff coverage is 30.26%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1141      +/-   ##
===========================================
- Coverage    65.28%   65.20%   -0.08%     
===========================================
  Files           81       81              
  Lines         8832     8853      +21     
===========================================
+ Hits          5766     5773       +7     
- Misses        3066     3080      +14     

@felixdivo felixdivo mentioned this pull request Dec 4, 2021
@felixdivo
Copy link
Collaborator

#1119 created some conflicts

@felixdivo
Copy link
Collaborator

@cowo78 I can do a review once they are resolved.

@cowo78 cowo78 mentioned this pull request Dec 10, 2021
Copy link
Collaborator

@felixdivo felixdivo left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution.

Is is possible that can/interfaces/ixxat/canlib_vcinpl2.py would need to be changed too?

can/interfaces/ixxat/canlib.py Show resolved Hide resolved
can/interfaces/ixxat/canlib_vcinpl.py Outdated Show resolved Hide resolved
can/interfaces/ixxat/canlib_vcinpl.py Outdated Show resolved Hide resolved
can/interfaces/kvaser/canlib.py Outdated Show resolved Hide resolved
can/interfaces/kvaser/canlib.py Outdated Show resolved Hide resolved
@felixdivo
Copy link
Collaborator

Am I right in that this only adds bus.state support for can/interfaces/ixxat/canlib_vcinpl1.py and not for can/interfaces/ixxat/canlib_vcinpl2.py?

@cowo78
Copy link
Collaborator Author

cowo78 commented Dec 22, 2021

Am I right in that this only adds bus.state support for can/interfaces/ixxat/canlib_vcinpl1.py and not for can/interfaces/ixxat/canlib_vcinpl2.py?

Correct. As mentioned in #1119 I would suggest to postpone a refactoring / code deduplication do another PR.

@felixdivo
Copy link
Collaborator

I'd like this to get a single other review before merging.

@felixdivo
Copy link
Collaborator

@hardbyte? 😃

@felixdivo felixdivo added this to the Next Release milestone Feb 9, 2022
@zariiii9003
Copy link
Collaborator

@cowo78, @fjburgos any objections to merging this?

Copy link
Owner

@hardbyte hardbyte left a comment

Choose a reason for hiding this comment

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

I've read through this patch while I don't have any issues with it I don't have the hardware to test it.

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

Successfully merging this pull request may close these issues.

None yet

4 participants