Skip to content
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.

Sign up for GitHub

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

Signal cleanup #99

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Signal cleanup #99

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
15bc06f
Block signals on widget restore and don't connect node multiple times.
syamajala Oct 22, 2024
23a464a
Fix save on new.
syamajala Oct 23, 2024
67aaae7
Destroy widget on close.
syamajala Oct 25, 2024
efb5ef7
Merge branch 'master' into signal_cleanup
syamajala Nov 14, 2024
d690ff6
Dont close filter on disconnect.
syamajala Nov 14, 2024
5a130cd
Fix filter widget.
syamajala Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions ami/client/flowchart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def update_title(filename):
title = 'AMI Client'
if hutch:
title += f' hutch: {hutch}'
if filename:
if filename is not None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated, but something that I noticed randomly. If you click new and then save it will save a chart as just '.fc'.

title += ' - ' + filename.split('/')[-1]

win.setWindowTitle(title)
Expand Down Expand Up @@ -172,6 +172,7 @@ def __init__(self, msg, broker_addr="", graphmgr_addr="", checkpoint_addr="", lo

self.ctrlWidget = self.node.ctrlWidget(self.win)
self.widget = None
self.connected = False

title = msg.name
if hutch:
Expand Down Expand Up @@ -230,9 +231,13 @@ def display(self, msg):
self.win.setCentralWidget(scrollarea)

if msg.state and hasattr(self.widget, 'restoreState'):
self.widget.blockSignals(True)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we do not block signals on the widget we cause a checkpoint to be emitted when restoring the state, which is unnecessary.

self.widget.restoreState(msg.state)
self.widget.blockSignals(False)

self.node.sigStateChanged.connect(self.send_checkpoint)
if not self.connected:
self.node.sigStateChanged.connect(self.send_checkpoint)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are connecting the sigStateChanged signal multiple times. Whenever you close the window and reopen it we reconnect which causes N checkpoints to be sent each time you change the widget.

self.connected = True

self.win.show()
if self.node.viewed:
Expand Down
2 changes: 1 addition & 1 deletion ami/flowchart/Flowchart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,7 @@ async def clear(self):
await self.chart.clear()
self.chartWidget.clear()
self.setCurrentFile(None)
self.chart.sigFileLoaded.emit('')
self.chart.sigFileLoaded.emit(None)
self.features = Features(self.graphCommHandler)
await self.graphCommHandler.updatePlots(self.features.plots)

Expand Down