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

Fix tray signals #812

Merged
merged 7 commits into from
Jun 28, 2024
Merged

Fix tray signals #812

merged 7 commits into from
Jun 28, 2024

Conversation

KIRA009
Copy link
Contributor

@KIRA009 KIRA009 commented Jun 27, 2024

What kind of change does this PR introduce?
This PR fixes the issues with signals not being passed around properly on windows

Summary
This PR replaces the QSocketNotifier with a QThread and a worker that uses a signal in addition to a parent/child pipe because in windows to pass data around. This additional maneouver was needed because windows wasn't triggered the activated signal of the qsocketnotifier object.

The current data flow is: child_conn -> parent_conn -> signal emit -> signal emit listener.

The signal emit listener is needed, because processing things in the Qthread object is not possible because of how Qt restricts updates from amywhere else other than the main thread. The signal emit listener is executed on the main thread, so the signal is used as a connection to send the data back to the main thread.

Checklist

  • My code follows the style guidelines of OpenAdapt
  • I have performed a self-review of my code
  • If applicable, I have added tests to prove my fix is functional/effective
  • I have linted my code locally prior to submission
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
  • New and existing unit tests pass locally with my changes

How can your code be run and tested?
Run the code (in development mode) in windows, and ensure that you see the notifications and the tray text changes.

Other information

@KIRA009 KIRA009 changed the title Fix/tray signals Fix tray signals Jun 27, 2024
@abrichr
Copy link
Member

abrichr commented Jun 27, 2024

Thank you @KIRA009 ! Please address the build issues when you get a chance so we can get this merged.

Also I noticed this includes the changes from https://github.com/OpenAdaptAI/OpenAdapt/pull/810/files, which itself has outstanding comments. How do you suggest we proceed?

@KIRA009
Copy link
Contributor Author

KIRA009 commented Jun 28, 2024

@abrichr I have updated both this and the application logs PR. We will have to merge the other PR first, before this gets merged

Copy link
Member

@abrichr abrichr left a comment

Choose a reason for hiding this comment

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

Thank you @KIRA009 🙏
LGTM!

@abrichr abrichr merged commit f231a34 into OpenAdaptAI:main Jun 28, 2024
1 check passed
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.

2 participants