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.

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

wants to merge 6 commits into from

Conversation

syamajala
Copy link
Contributor

Trying to track down #83 this fixes two related issues but still does not solve the problem yet.

@syamajala syamajala requested a review from vespos October 24, 2024 17:14
@@ -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'.

@@ -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.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.

@syamajala
Copy link
Contributor Author

I broke something in this pull request.

@syamajala
Copy link
Contributor Author

Actually I guess I havent pushed those changes yet, the pull request as it is, is ok.

@@ -633,7 +633,7 @@ def __init__(self, name):

def display(self, topics, terms, addr, win, **kwargs):
if self.widget is None:
self.widget = FilterWidget(terms, self.output_vars(), win)
self.widget = FilterWidget(terms or self.input_vars(), self.output_vars(), win)
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 the fix for the filter. The state is getting out of sync between the flowchart and the subprocess. The subprocess calls display with all the arguments as none, but the subprocess has them filled in, which causes the inconsistency in state.

Maybe a better fix would be to call display with the terms argument in build_views.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant