Allow multiple flow controllers to be connected on the same port

numat / alicat

Python driver and command line tool for Alicat mass flow controllers.

GNU General Public License v2.0

21 stars 27 forks source link

Allow multiple flow controllers to be connected on the same port #4

Closed acowsik closed 6 years ago

patrickfuller commented 6 years ago

Thanks for the pull request! A couple of comments:

Can you provide a use case? Is this using multiple devices with the Alicat addressing?
What purpose is self.open serving? I don't see it ever getting toggled to False.

acowsik commented 6 years ago

I edited the readme to provide an example for using multiple controllers. Alicat provides a multiplexing device that allows multiple controllers to end up on the same USB port which I use for mixing gasses together. self.open should have toggled at the end of the FlowController.close - and now it stops read/write on the controller if it has been closed. This is the first time I've tried to contribute to anything I didn't own on github so I'm not sure if the commits I've pushed will show up or if I need to do something more.

patrickfuller commented 6 years ago

Looks good! A couple of minor edits and this is good to go.

Remove trailing whitespace. Seems minor, but trailing whitespace makes git diffs messy.
Rename OpenPorts to open_ports to match the snake case of the rest of the class. Also replace the """ docstring comment above it with a couple lines of # comments per PEP8.
Format your new docstrings per PEP257. One-line summary, blank line, more info.

Let me know if you have any questions!

acowsik commented 6 years ago

I think all the style points that you mentioned are in order now. I'm happy that you helped me with this so far.

patrickfuller commented 6 years ago

Looks good. Thanks for the pull request!

patrickfuller commented 6 years ago

I updated the version, so pip install --upgrade alicat should work on your end. Let me know if you have any issues.