CampbellGroup / common

Shared campbell lab code.
GNU Lesser General Public License v3.0
8 stars 5 forks source link

Amj/serial server cg #97

Closed jayich closed 8 years ago

jayich commented 8 years ago

Review: @aransfor , @theoriginaljuice , @gregllong

Along the same lines as https://github.com/CampbellGroup/common/pull/94

jayich commented 8 years ago

cxn.serial_server does not appear in common. Perhaps this is not too surprising as we always interact with serial_server via a node, so magic_serial_server will be in molecules and that will need to be changed to magic_serial_server_cg.

aransfor commented 8 years ago

can you clarify? cxn.serial_server is a python object, do you mean that that serial_server.py is not in common?

jayich commented 8 years ago

Sorry, the string serial_server does not appear in any files, and that search would find cxn.serial_server, for example. Please double check. I'm searching molecules now for node specific instances of serial_server.

We should consider renaming the file to serial_server_cg.py.

jayich commented 8 years ago

I think the main fix is 'Serial Server' in the node script to 'Serial Server cg'.

gregllong commented 8 years ago

LGTM

jayich commented 8 years ago

We are set to merge. There was only the node script that we needed to fix in our repository.

aransfor commented 8 years ago

Ok, youll have to give me another day to test this as alot of our code uses the serial server and I am kind of swamped right now

jayich commented 8 years ago

Sure thing. I think you'll be surprised to find that the actual instances of 'serial_server' appearing in your code is quite small. data_vault is going to be a different story.

aransfor commented 8 years ago

so the specifics of why serial_server is never called is in serialdeviceserver, it searches for ANY servers with serial in the name and looks for com ports on those servers. see this gist https://gist.github.com/aransfor/b654d7f66ea617781cfc49d613c9724a

jayich commented 8 years ago

excellent.

barium-project commented 8 years ago

LGTM. We don't even use the node server yet so there should be no conflicts for us.

aransfor commented 8 years ago

LGTM