Skip to content

Add hardware handshake support for serial port based CAN interfaces#402

Merged
felixdivo merged 2 commits intohardbyte:developfrom
yegorich:serial-hc
Sep 4, 2018
Merged

Add hardware handshake support for serial port based CAN interfaces#402
felixdivo merged 2 commits intohardbyte:developfrom
yegorich:serial-hc

Conversation

@yegorich
Copy link
Copy Markdown
Contributor

@yegorich yegorich commented Aug 31, 2018

pySerial module provides 'rtscts' option to handle hardware handshake.
This setting will be extracted from init's kwargs.

Example:

bus = can.interface.Bus(bustype='slcan',
                                      channel='/dev/ttyUSB0@3000000',
                                      bitrate=1000000,
                                      rtscts=True)

Closes #398

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 31, 2018

Codecov Report

Merging #402 into develop will decrease coverage by <.01%.
The diff coverage is 75%.

@@             Coverage Diff             @@
##           develop     #402      +/-   ##
===========================================
- Coverage    59.26%   59.25%   -0.01%     
===========================================
  Files           55       55              
  Lines         4262     4261       -1     
===========================================
- Hits          2526     2525       -1     
  Misses        1736     1736

Copy link
Copy Markdown
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.

The rtscts param should be documented in the docstrings.

raise ValueError("Must specify a serial port.")

self.channel_info = "Serial interface: " + channel
rtscts = kwargs.get('rtscts', False)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This could be done simpler and easier to read by making this a keyword argument of the __init__() method, like with baudrate or timeout. Same applies for the other constructor.

Copy link
Copy Markdown
Contributor Author

@yegorich yegorich Sep 1, 2018

Choose a reason for hiding this comment

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

pylint was complaining about too many call parameters in __init__, so I decided to use kwargs. Should I make it a keyword argument anyway?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, I don't see the problem with more parameters ... makes it way easier to spot the options and defaults.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK. Will do.

pySerial module provides 'rtscts' option to handle hardware handshake.
This setting will be extracted from __init__'s kwargs.

Example:

    bus = can.interface.Bus(bustype='slcan',
                            channel='/dev/ttyUSB0@3000000',
                            bitrate=1000000,
                            rtscts=True)
@yegorich
Copy link
Copy Markdown
Contributor Author

yegorich commented Sep 3, 2018

v2: move rtscts argument to the parameter list and also document it.

@felixdivo felixdivo merged commit 0b2f31d into hardbyte:develop Sep 4, 2018
@yegorich yegorich deleted the serial-hc branch September 4, 2018 11:44
@yegorich
Copy link
Copy Markdown
Contributor Author

yegorich commented Sep 5, 2018

Are you planning to resolve all 8 remaining issues for 2.3 or there could be an earlier release?

@felixdivo
Copy link
Copy Markdown
Collaborator

@hardbyte What do you think?

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