8000 cleanups,fixes & docs in Bus.__new__() and util.load_config() by felixdivo · Pull Request #309 · hardbyte/python-can · GitHub
[go: up one dir, main page]

Skip to content

cleanups,fixes & docs in Bus.__new__() and util.load_config()#309

Merged
felixdivo merged 6 commits intodevelopfrom
fix-280
May 28, 2018
Merged

cleanups,fixes & docs in Bus.__new__() and util.load_config()#309
felixdivo merged 6 commits intodevelopfrom
fix-280

Conversation

@felixdivo
Copy link
Collaborator

This PR changes util.load_config() to pass through any unused parameters. It is made more clear in the code what Bus.__new__() is doing and what not. A lot has moved to util.load_config() to remove code and responsibility duplication. The documentation has each been changed to reflect what the methods are actually doing, that was not done correct before.

Based an @randycoulman's comment over here
Fixes #280

@felixdivo felixdivo self-assigned this May 16, 2018
@felixdivo felixdivo requested review from christiansandberg and hardbyte and removed request for hardbyte May 16, 2018 17:16
@felixdivo
Copy link
Collaborator Author

This is done, but we should write some unit tests before merging this PR to later prevent regression.

Note: Making __new__ a @classmethod was a really nasty bug and took me quite a while to figure out. -.- Don't do this in the future! 😛

@felixdivo felixdivo requested a review from hardbyte May 27, 2018 17:28
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.

Looks good to me. Just two debugging print statements to remove.

can/interface.py Outdated

# the channel attribute should be present in **config
print("DEBUGGING: ", args)
print("DEBUGGING: ", config)
Copy link
Owner

Choose a reason for hiding this comment

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

Remove these (or convert to debug level logs)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah sure, must've forgotten them there.

@felixdivo felixdivo added this to the 2.2 Release milestone May 27, 2018
@felixdivo felixdivo merged commit 6ebe610 into develop May 28, 2018
@felixdivo felixdivo deleted the fix-280 branch May 28, 2018 05:46
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

0