-
Notifications
You must be signed in to change notification settings - Fork 83
add HHS resource model #434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
from pylabrobot.resources.coordinate import Coordinate | ||
|
||
|
||
def hamilton_heater_shaker(name: str, shaker_index: int): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far we always captitalised the first letter for resource definition functions (to showcase that they return a class?).
Should we maintain this here and make this Hamilton_heater_shaker
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in python it's idiomatic to make function names lower case, regardless of return type. i think we should adhere to that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know but the consequence is then that we should rename every entry in the PLR resource library?
|
||
class HamiltonHeatShakerBackend(HeaterShakerBackend): | ||
""" | ||
Backend for Hamilton Heater Shaker devices connected through an Heater Shaker Box |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also say that 2 Hamilton_heater_shaker
s can be used via directly connecting them physically to the RS232 ports and controlling them via the STAR()
backend?
But in that case this resource
definition would still work as a standalone resource
as long as the .setup()
method is not called on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that is possible. we should mention it
we haven't been able to figure out how that works though. one time when i tried i had to factory reset all software on the machine. I think if people start doing that, we would still have to have some way for users to control the machine (eg set_temperature) which is not possible without the appropriate backend. i suggest making this decision later when we actually figure out how to connect an HHS to the star directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been using our HHSs with direct connection to the STAR since I integrated it in PR#66: Direct device integration: Hamilton Heater-Shaker (HHS) & Hamilton Heater-Cooler (HHC) - Part 1: device control
Should we then figure this out now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think cleaning that up can be a follow-up PR. Let's keep this one for renaming the backend and introducing a resource function.
size_y=103.8, | ||
size_z=74.11, | ||
backend=HamiltonHeatShakerBackend(shaker_index=shaker_index), | ||
child_location=Coordinate(x=9.66, y=9.22, z=74.11), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How have you made these measurements?
I've got child_location=Coordinate(x=9.25, y=8.5, z=76.1),
I'll check them right now again, my approach:
- 3D printed a perfectly cuboidal SLAS-compliant "calibration plate block"
- open HHS
- place "calibration plate block" onto HHS
- make HHS close + shake for 5 seconds + open again ->final position of "calibration plate block" is the real origin of the child location (in
x
andy
dimension)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
caliper and then iswap movements 🤡
how do you get the final position?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the child_location discrepancy might be related to the orbit diameter, this was calibrated with a 3mm orbit shaker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ours is also a 3mm orbit shaker.
how do you get the final position?
Explained here:
my approach:
- 3D printed a perfectly cuboidal SLAS-compliant "calibration plate block"
- open HHS
- place "calibration plate block" onto HHS
- make HHS close + shake for 5 seconds + open again ->final position of "calibration plate block" is the real origin of the child location (in
x
andy
dimension)
Then using a teaching needle to move to the origin x-y coordinate of the "calibration plate block".
I have done this 1 year ago though and been using it since then.
I will remeasure tomorrow :)
acceleration: increments per second | ||
""" | ||
int_speed = int(speed) | ||
assert 20 <= int_speed <= 2_000, "Speed must be between 20 and 2_000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think this is universally correct:
I believe we can disregard the adapters that can be attached to the top of the Hamilton_heater_shaker for the sake of machine control.
But we should distinguish between physical differences: It seems to me that actually there are 3 different Hamilton_heater_shaker models, classified based on their shaking_orbit_distance
:
- 1.5 mm
- 2.0 mm
- 3.0 mm
What is unclear is whether this actually changes the max RPM that these machines can achieve?
Having different adapters change the max safe RPM makes sense, but physically it shouldn't limit each Hamilton_heater_shakers capabilities... I think we should discuss how to implement this efficiently here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh this is very interesting. We actually arbitrarily take off adapters (just allen wrench) and put them on other heater shakers 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not restrain the shaker below the RPM that it can actually reach: the highest RPM from this list is 2500 RPM.
We can place these recommendations into the docstring:
v1 proposal:
def Hamilton_HS(name: str) -> PlateHolder:
"""
Hamilton cat. no.: 199033, 199034, 199034
- manufacturer_link:
- temperature control = RT+5°C - 105°C (all variants),
- notes:
- Hamilton Heater Shaker, aka HHS, with flat bottom adapter,
- variants:
- 199027:
- shaking orbit = 1.5mm,
- shaking speed = 100 -1800 rpm,
- 199033:
- shaking orbit = 2.0mm,
- shaking speed = 100 -2500 rpm,
- 199034:
- shaking orbit = 3.0mm,
- shaking speed = 100 -2400 rpm,
- max. loading = 500 grams
"""
return PlateHolder(
name=name,
size_x=145.5,
size_y=104.0,
size_z=184.1-8.0-100, # includes HHS' carrier_site_skirt_height=2.85mm
# probe height - carrier_height - deck_height
child_location=Coordinate(9.25, 8.5, 184.1-8.0-100),
model="Hamilton_HS",
pedestal_size_z=0
)
temp_str = f"{round(10*temperature):04d}" | ||
return self._send_command("TA", ta=temp_str) | ||
|
||
async def get_current_temperature(self) -> float: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh nice, they are also available on the direct interface. the firmware api appears to be exactly the same
Multiple Hamilton Heater Shakers can be connected to the same Heat Shaker Box. Each has A | ||
separate 'shaker index' | ||
""" | ||
assert shaker_index >= 0, "Shaker index must be non-negative" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a limit to how many shakers can be connected to one HeaterShakerBox connection?
If yes, should this assertion ensure this is adhered to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! there are only 8 power/data plugs on the box, but the hhs themselves can be set to any index between 0 and 9. index 0 is the one that gets a direct USB connection
"""get temperature in Celsius""" | ||
response = self._send_command("RT").decode("ascii") | ||
temp = str(response).split(" ")[1].strip("+") | ||
return float(temp) / 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this should be updated to return both values?
It looks like it only returns the edge temperature at the moment?
No description provided.