-
Notifications
You must be signed in to change notification settings - Fork 6
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
Robot firefly #221
Robot firefly #221
Conversation
The tests in pull_request are so slow, not finished even after 8 hours. |
@yanna yes there is a problem with the tests where they finish but don't return to the shell so never count as complete. I'm working on it.
Sent from Proton Mail Android
…-------- Original Message --------
On 5/29/24 07:27, yannachen wrote:
The tests in pull_request are so slow, not finished even after 8 hours.
—
Reply to this email directly, [view it on GitHub](#221 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AASBDGCMZDD35VWIGAJJMYTZEXCS7AVCNFSM6AAAAABIOCIHKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZXGI4TANBZG4).
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
I cancelled the workflow because it finished the tests but didn't exit the test runner. There was 1 failure, so you could take a look while a fix the test runner (hopefully). |
I think you may have the wrong person. I'm not Yanna Chen which I believe
is the one you want.
…On Wed, May 29, 2024, 9:43 AM Mark Wolfman ***@***.***> wrote:
I cancelled the workflow because it finished the tests but didn't exit the
test runner. There was 1 failure, so you could take a look while a fix the
test runner (hopefully).
—
Reply to this email directly, view it on GitHub
<#221 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHSPYS2C26WAHYPHFPZ753ZEYAUZAVCNFSM6AAAAABIOCIHKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZXHA2DONZZG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Absolutely right. Sorry about 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.
Good start, nothing major. I think most of the changes are removing commented-out code blocks or leftover print statements from development.
src/firefly/application.py
Outdated
@@ -57,6 +61,7 @@ class FireflyApplication(PyDMApplication): | |||
show_voltmeters_window_action: QtWidgets.QAction | |||
show_logs_window_action: QtWidgets.QAction | |||
launch_queuemonitor_action: QtWidgets.QAction | |||
# show_robot_window_action: QtWidgets.QAction |
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 be removed if not needed.
src/firefly/application.py
Outdated
@@ -696,6 +710,12 @@ def show_sample_viewer_window(self): | |||
FireflyMainWindow, ui_dir / "sample_viewer.ui", name="sample_viewer" | |||
) | |||
|
|||
# @QtCore.Slot() |
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.
Remove if not needed.
src/firefly/main_window.py
Outdated
@@ -140,6 +140,9 @@ def customize_ui(self): | |||
text="Sample", | |||
menu=self.ui.positioners_menu, | |||
) | |||
# Robot transfer sample |
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.
Remove if not needed.
src/firefly/robot.py
Outdated
|
||
# ROBOT_NAMES = ["Austin"] | ||
|
||
SAMPLE_NUMBERS = [8, 9, 10, 14, 15, 16, 20, 21, 22, None] # list(range(24))+[None] |
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.
We shouldn't hard-code this list of which samples are enabled in the UI layer. Can we get this information from the Robot()
device somehow?
src/firefly/robot.py
Outdated
for i in range(num): | ||
layout = self.regions[-1].layout | ||
# iterate/wait, and delete all widgets in the layout in the end | ||
while layout.count(): |
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.
Could this be while layout.count() > 0
? This current way should work okay, but checking explicitly for > 0
makes it more clear what you're thinking.
src/firefly/tests/test_robot_gui.py
Outdated
assert len(disp.regions) == 1 | ||
|
||
|
||
from ophyd import Component as Cpt |
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.
Move to top of file with other ophyd imports.
src/firefly/tests/test_robot_gui.py
Outdated
def sim_motor_registry(sim_registry): | ||
# Create the motors | ||
motor1 = FakeHavenMotor(name="motor1") | ||
sim_registry.register(motor1) |
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 don't think you need to explicitly register devices anymore. Do the tests pass without these two sim_registry.register()
calls?
src/firefly/tests/test_robot_gui.py
Outdated
|
||
# get macros | ||
macros = display.macros() | ||
print(macros) |
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.
Remove print statement.
No description provided.