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

Add shush-like functionality #13

Merged
merged 5 commits into from
Mar 10, 2017
Merged

Conversation

adamyonk
Copy link
Contributor

This is a work-in-progress and just a proof-of-concept right now.

I wanted to see if I could copy the functionality of Shush.app with Hammerspoon - turns out it was not that difficult. Basically, you hold a hot key to mute or unmute the mic, and double-tap the hot key to toggle hold-to-mute or hold-to-unmute. It's a super minimal app that just brings push-to-talk to macOS.

screen shot 2017-01-30 at 4 33 20 pm

Building out the hotkey functionality is kind of funny in Hammerspoon - from what I could tell you can't use hs.hotKey or hs.hotKey.modal to listen for meta-only keys (like ⌥), so I ended up doing basically the same thing you did in control-escape.lua.

I got all the way to this point and realized that hs.audiodevice:setMuted(state) doesn't exactly work as advertised. I can see the current device being muted/unmuted in Audio MIDI Setup.app, but it still comes through in all of the applications I tested in.

screen shot 2017-01-30 at 4 41 00 pm

It sounds like another option is to set the input volume to 0, but I've read that it doesn't completely mute the input, but just makes it really quiet. I'm going to try that next, but I just wanted to put this here to see if it is something you're interested in (and to see if maybe someone knows why setMuted doesn't actually work 😄).

@jasonrudolph
Copy link
Owner

Nice. I hadn't considered anything like this before, but I ❤️ the idea of having a universal keystroke that provides mute/push-to-talk in every app.

@adamyonk adamyonk force-pushed the microphone branch 5 times, most recently from 3e928dd to 1b5c730 Compare March 8, 2017 15:26
@adamyonk
Copy link
Contributor Author

adamyonk commented Mar 8, 2017

So I tried this out again, I'm not sure what my deal was before, but I think it's working! If anyone would like to try it out and report back, I think it's ready to move forward.

I set the key to ⌥. When you reload your config, it should show a little alert that says if the mic is hot or muted.

  • While the mic is hot, holding ⌥ will mute, while muted, opposite
  • Double-tapping ⌥ will toggle muted/hot and show a notification

Copy link
Owner

@jasonrudolph jasonrudolph left a comment

Choose a reason for hiding this comment

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

This is slick, @adamyonk! ⚡ I tested this out locally and it works advertised. Thanks for this contribution!

I've included a few questions and requests in the code review below. If you have any questions, just let me know.


local messageMuting = message.new('muted 🎤')
local messageHot = message.new('hot 🎤')
local last_mods = {}
Copy link
Owner

Choose a reason for hiding this comment

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

I think the snake_case here comes from the snake_case used in control-escape.lua. Truth be told, control-escape.lua only uses snake_case because I haven't gotten around to changing it to the more idiomatic camelCase naming used everywhere else in this repo. 😇

Would you mind changing these snake_case names to camelCase?

recently_clicked = false
end

control_key_timer = hs.timer.delayed.new(0.3, control_key_handler)
Copy link
Owner

Choose a reason for hiding this comment

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

I suspect that the name control_key_timer is copy-and-paste from control_escape.lua. Since we're dealing with the option key here, I think optionKeyHandler would be a more intention-revealing name.

end
display_status()

toggle = function (device)
Copy link
Owner

Choose a reason for hiding this comment

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

Tiny style-related request: Remove the whitespace between function and ( for consistency with the existing code.

local recently_clicked = false
local second_click = false

display_status = function ()
Copy link
Owner

Choose a reason for hiding this comment

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

Tiny style-related request: Remove the whitespace between function and ( for consistency with the existing code.

notify = function(self, seconds)
local seconds = seconds or 1
self:show()
hs.timer.delayed.new(seconds, function () return self:hide() end):start()
Copy link
Owner

Choose a reason for hiding this comment

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

Tiny style-related request: Remove the whitespace between function and ( for consistency with the existing code.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think the return is required here. If it's not required, would you mind removing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right!

@@ -34,6 +34,7 @@ require('control-escape')
require('delete-words')
require('hyper')
require('markdown')
require('microphone')
Copy link
Owner

Choose a reason for hiding this comment

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

Alphabetical order 😻

control_key_timer = hs.timer.delayed.new(0.3, control_key_handler)

option_handler = function(event)
local device = hs.audiodevice.current(true).device
Copy link
Owner

Choose a reason for hiding this comment

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

Would it make sense to use hs.audiodevice.defaultInputDevice() here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think I was taking the long way around here.

@adamyonk
Copy link
Contributor Author

adamyonk commented Mar 9, 2017

Changes made, thanks for the feedback! Let me know if there's anything else you want tweaked.

@jasonrudolph jasonrudolph merged commit 5ba80d5 into jasonrudolph:master Mar 10, 2017
@jasonrudolph
Copy link
Owner

Changes made

Thanks, @adamyonk. You rock! 🤘

@adamyonk
Copy link
Contributor Author

Yeah, thanks for the additional cleanup! Looks like I still missed a few!

@jasonrudolph
Copy link
Owner

This is cool enough that it deserves its own section in the README, but I didn't want to hold up sharing this goodness with the world. If you're up for it, it would be awesome to have a PR to add a section to the README describing this feature. 😁

jasonrudolph added a commit that referenced this pull request Mar 10, 2017
@jasonrudolph
Copy link
Owner

After using this for a couple weeks, I'm digging the functionality, but I'm struggling with the keybinding. While on a video chat, I'm often talking and typing at the same time, either for pairing or for taking notes during a meeting. And since I often hit the option key while typing, my audio occasionally gets muted unintentionally while I'm talking. @adamyonk: Have you run into this at all?

So far, I'm thinking that I could resolve this issue by changing the keybinding to a key that I use less frequently than the option key. I pushed up a branch that uses the fn key (b05b17e). I'm gonna try that for a while and see how it feels.

@bkeepers
Copy link

And since I often hit the option key while typing, my audio occasionally gets muted unintentionally while I'm talking. @adamyonk: Have you run into this at all?

This happens to me on pretty much every video call. It might help to increase the delay from 0.3, but even then I think it's common to hold option for longer than that since you're often pressing another key with it.

👍 for fn

@jnunemaker
Copy link
Contributor

Agreed. I hit option all the time to jump words left and right and have been doing a lot of muting/unmuting. 😄

@adamyonk
Copy link
Contributor Author

adamyonk commented Mar 29, 2017 via email

@jasonrudolph
Copy link
Owner

And since I often hit the option key while typing, my audio occasionally gets muted unintentionally while I'm talking. ... I'm thinking that I could resolve this issue by changing the keybinding to a key that I use less frequently than the option key. I pushed up a branch that uses the fn key (b05b17e).

Thanks for the discussion. #21 changes the keybinding from option to fn.

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.

4 participants