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

Initial meeting tag support #1172

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

Conversation

pokey
Copy link
Collaborator

@pokey pokey commented Apr 15, 2023

Comment on lines 35 to 41
app.notify("Mute meeting")

def meeting_unmute():
app.notify("Unmute meeting")

def meeting_exit():
app.notify("Exit meeting")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nriley here's where you'd put the implementations of mute / unmute / exit. Need to decide whether we're comfortable using accessibility here. Also prob need to make the tag mac-specific if we use accessibility



def is_zoom(app):
return app.bundle == "us.zoom.xos"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Todo: make this work on Windows

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should rename this file? Because this file name should really be for when Zoom is focused

@pokey
Copy link
Collaborator Author

pokey commented Apr 15, 2023

@nriley feel free to push directly to this branch, or open a PR against it if you can't for whatever reason

Also adds user.meeting_is_muted.

Tested in US English, German and Simplified Chinese localizations.
@nriley
Copy link
Collaborator

nriley commented Apr 16, 2023

Instead of looking at the menu (with a different title per-language) I can recognize the shape of the "you are muted" icon and determine the mute status this way; it worked across the localizations and meeting window layouts I tried. Still might be some issues (even ignoring the Talon bug) in recognizing the meeting window title.

If we just wanted to support toggling mute, we could do it without accessibility.

I put in a partial workaround for the bug by focusing another app and back to Zoom. I don't think actually checking the 2nd time should help in most cases but it should help Talon to notice the window for next time. The problem is still though that the tag won't activate, so the voice commands don't do anything (other than "media mute" as @pokey experienced yesterday :).

One thing to note while I was testing, which in part inspired my two-microphone solution: it seems that if you mute audio in Zoom, you can't talk to Talon either on the same microphone (works fine with another microphone).

Another action that might be helpful would be to allow you to focus the active meeting window since I've certainly had cases where it gets "lost". This wouldn't require accessibility either (at least for Zoom), since my window filtering just uses the title.

@nriley
Copy link
Collaborator

nriley commented Apr 16, 2023

Here's a few options of more robust workarounds for the problem of meeting commands not being active since Talon hasn't "seen" the meeting window yet.

We could have the meeting commands active if any of the meeting apps are active, using "user.running". Then in the default implementation for "meeting mute", etc., we could "poke" each of the apps (via accessibility to find the window) to see if we're failing to notice an active meeting window.

Another option is to poll windows in active meeting apps by accessibility (which I may end up doing in my own repo, so I can accurately switch microphones) and "force" Talon to notice any it hasn't found. I do that in one of my private repos in a specific circumstance, but I'd be a bit worried about doing that by default.

What would be great is if the bug could be fixed and I could rip out all these workarounds :-)

Could not figure out any way to determine mute status that isn't language-specific or talon.experimental.locate, so just notify if we can't figure it out. "Meeting mute" will toggle mute/unmute regardless.
@nriley
Copy link
Collaborator

nriley commented Apr 22, 2023

Added Teams support — due to its use of Electron this is more complex/asynchronous. Given Teams is pending a rewrite I'm disinclined to spend much more time on this particular implementation, but it at least works for me. Talon seems to be more reliably "see" the meeting window in Teams, so I didn't include the same workaround I did for Zoom.

This allows you to react to starting/stopping a meeting by overriding these actions, and using actions.next to invoke the default implementation.

Also, remove redundant check for a meeting window in Teams and Zoom on Talon launch.
@nriley
Copy link
Collaborator

nriley commented Apr 22, 2023

Refactored a bit to put the user.meeting_appname tag context in one place rather than creating a context per meeting app. At this point we could also just add the user.meeting tag in the module implementation rather than using .talon files. I left that part alone for now as I don't feel strongly one way or the other.

Here's where I override the user.meeting_started/user.meeting_ended in my suspend script to turn off or switch the microphone during a meeting: nriley@26ac780

@auscompgeek
Copy link
Collaborator

I can recognize the shape of the "you are muted" icon and determine the mute status this way

Does this work even when the Zoom meeting controls are set to auto-hide?

@nriley
Copy link
Collaborator

nriley commented Apr 24, 2023

I can recognize the shape of the "you are muted" icon and determine the mute status this way

Does this work even when the Zoom meeting controls are set to auto-hide?

Yes, I think so — the icon I am matching shows up even if the controls are hidden.

image

If you can find cases where the above mute icon does not appear, then I can implement checking for the mute menu item in addition (like I do on Webex) — unfortunately this will not be robust to non-English UI languages.

@auscompgeek
Copy link
Collaborator

Yes, I think so — the icon I am matching shows up even if the controls are hidden.

Hmm, that looks like the one that'd show up on your self-view in the gallery view? Wouldn't that:

  1. not be shown when your self-view is hidden?
  2. also have false positive matches with other muted people in the gallery on the same call?

@nriley
Copy link
Collaborator

nriley commented Apr 24, 2023

Might be. For obvious reasons (only one of me) it's been a bit hard to test. If you have any other thoughts on how to detect when Zoom is muted (particularly in a way that does not require textual matching of localized UI elements) happy to hear them.

@nriley
Copy link
Collaborator

nriley commented Apr 28, 2023

OK, I've moved off my slick-but-broken Zoom mute detection to a method that works in meetings with others joined (yeah, yeah…) and with Self View disabled but is dependent on the UI being in English.

More testing appreciated!

@auscompgeek
Copy link
Collaborator

Does the Zoom menu bar item need to be enabled for that to work, or does it use the menus for the meeting window?

@nriley
Copy link
Collaborator

nriley commented Apr 29, 2023

Yes, it does not use the menu in the top right corner of the screen but instead uses the application menus even if the application is not active.

nriley added 2 commits May 18, 2023 10:35
… meeting.

Also better explain why each identifier is used - the same window is shared with "pre-joining", but if you are switching between the smaller and larger versions of the meeting window, the first identifier exists in both.

(Suspect the prior identifier is no longer being used as of Teams 1.6.00.11156.)
@nriley
Copy link
Collaborator

nriley commented Jun 17, 2023

Since I upgraded either to a recent Talon beta or macOS (or both), this has become incredibly unreliable. (It's blocked on the window activation callback issue in core Talon, but it was at least usable previously.) Will work on fixing it as I have time.

@nriley
Copy link
Collaborator

nriley commented Jun 25, 2023

talonvoice/talon#606 is fixed in the latest beta! I tested Zoom and it seems reliable.

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.

3 participants