Skip to content

Misc cleanups#367

Merged
felixdivo merged 12 commits intodevelopfrom
misc-cleanups
Jul 19, 2018
Merged

Misc cleanups#367
felixdivo merged 12 commits intodevelopfrom
misc-cleanups

Conversation

@felixdivo
Copy link
Copy Markdown
Collaborator

@felixdivo felixdivo commented Jul 18, 2018

I cleaned up some parts of the library, and reduced the number of Sphinx doc warnings from 43 (or 55 depending on what you compare it with) to 39. Fixing some warnings created new ones.

Related to #262

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 18, 2018

Codecov Report

Merging #367 into develop will increase coverage by 0.06%.
The diff coverage is 76.47%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #367      +/-   ##
===========================================
+ Coverage    58.26%   58.32%   +0.06%     
===========================================
  Files           54       54              
  Lines         4248     4250       +2     
===========================================
+ Hits          2475     2479       +4     
+ Misses        1773     1771       -2
Impacted Files Coverage Δ
can/interfaces/ixxat/canlib.py 18.28% <ø> (+0.06%) ⬆️
can/CAN.py 0% <ø> (ø) ⬆️
can/interfaces/slcan.py 22.5% <ø> (ø) ⬆️
can/bus.py 79.26% <ø> (ø) ⬆️
can/interfaces/usb2can/serial_selector.py 0% <0%> (ø) ⬆️
can/notifier.py 93.33% <100%> (ø) ⬆️
can/io/asc.py 96.55% <100%> (+0.02%) ⬆️
can/thread_safe_bus.py 66.66% <80%> (+1.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e66d6d...b73f70d. Read the comment docs.

@felixdivo felixdivo requested a review from hardbyte July 19, 2018 06:53
Copy link
Copy Markdown
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 just a couple of small points


A thread safe bus wrapper is also available, see `Thread safe bus`_.

Autoconfig Bus
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why moving this autoconfig bus before the BusABC?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because that is the main entry point into the library if you want to communicate.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Well it is a wrapper/constructor around a concrete implementation of the BusABC - both should be covered and I assumed that the more interesting/important item was the class that acts as the primary interface.

Leave your changes - we should just add a clarifying note to the top to help explain the class structure to users.

For example use ``/dev/ttyUSB0@115200`` or ``COM4@9600``

.. note:
An Arduino-Interface could easyly be build wit this:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Two typos: easily and with

@felixdivo felixdivo merged commit 1c04905 into develop Jul 19, 2018
@felixdivo felixdivo deleted the misc-cleanups branch July 19, 2018 13:59
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