Skip to content

Added status_string method to return simple status strings#725

Merged
felixdivo merged 8 commits intohardbyte:developfrom
tamenol:status_string
Feb 5, 2020
Merged

Added status_string method to return simple status strings#725
felixdivo merged 8 commits intohardbyte:developfrom
tamenol:status_string

Conversation

@tamenol
Copy link
Contributor

@tamenol tamenol commented Nov 14, 2019

status_string returns the current status of the bus in a single word.

Ok, so I have no idea this is warrented to get merged. I wrote this function as I needed a single word for the status of the bus (just like you have in PCAN-View). And I didn't want to import .basic in my project to get the needed values, hence why I put it in the PcanBus class

status_string returns the current status of the bus in a single word
@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

Merging #725 into develop will increase coverage by 0.24%.
The diff coverage is 12.5%.

@@             Coverage Diff             @@
##           develop     #725      +/-   ##
===========================================
+ Coverage    66.72%   66.96%   +0.24%     
===========================================
  Files           70       70              
  Lines         6398     6499     +101     
===========================================
+ Hits          4269     4352      +83     
- Misses        2129     2147      +18

elif status == PCAN_ERROR_BUSOFF:
return "BUSOFF"
else:
return "NonImplementedError"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I' return None in this case, to make it more explicit that something went wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good idea. However, when looking back on this, wouldn't it be better if I made a dictonary with all the errors and their strings in basic.py and retrieve them with this function, so the class in pcan.py doesn't get cluttered much?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felixdivo see commit 890128f

@felixdivo
Copy link
Collaborator

Sorry for the long delay, do you still have tome to fix this?

@tamenol
Copy link
Contributor Author

tamenol commented Jan 30, 2020

@felixdivo No problem. And yes, I'll have a look this weekend/in the course of next week.

@tamenol tamenol requested a review from felixdivo February 4, 2020 12:31
@felixdivo
Copy link
Collaborator

Looks good to me.

@felixdivo
Copy link
Collaborator

Except the formatter is complaining here. You can fix it by executing black . in python-can/can/.

@felixdivo
Copy link
Collaborator

What a massive change 👍

@felixdivo felixdivo merged commit e495478 into hardbyte:develop Feb 5, 2020
@tamenol tamenol deleted the status_string branch February 5, 2020 14:28
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.

2 participants