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

Jump to bottom

Support M1 Macs using DisplayServicesGetBrightness/DisplayServicesSetBrightness #36

Merged
merged 1 commit into from
Feb 28, 2021
Merged

Support M1 Macs using DisplayServicesGetBrightness/DisplayServicesSetBrightness #36

merged 1 commit into from
Feb 28, 2021

Conversation

jtbandes
Copy link
Contributor

I found that these SPI functions work to get and set the brightness on my M1 MacBook Air. Fixes #34.

I have not tested this with any other system/configuration, so will need help from others to test thoroughly.

  • Extracted functions from the main loop to avoid it getting too complicated.
  • Also made minor changes to fix a warning about brightness being uninitialized.

@jtbandes
Support M1 Macs using DisplayServicesGetBrightness/DisplayServicesSet…
4270134 
…Brightness

- Tested on a M1 MacBook Air running Big Sur, for the built-in display.
- Extracted functions from the main loop to avoid it getting too complicated.
- Also made minor changes to fix a warning about `brightness` being uninitialized.
@jtbandes jtbandes mentioned this pull request Feb 22, 2021
Closed
@nriley
Copy link
Owner

nriley commented Feb 22, 2021

Thanks! Testing help appreciated.

nriley referenced this pull request in Hammerspoon/hammerspoon Feb 22, 2021
@cmsj
Add a note to hs.brightness about it being essentially useless on M1 …
cededda 
…Macs. Closes #2668
Mike-Woolley added a commit to Mike-Woolley/brightness that referenced this pull request Feb 28, 2021
@Mike-Woolley
Merged in PR "Support M1 Macs using DisplayServicesGetBrightness/Disp…
f192e9d 
…layServicesSetBrightness" nriley#36.
Mike-Woolley added a commit to Mike-Woolley/brightness that referenced this pull request Feb 28, 2021
@Mike-Woolley
Merged in PR nriley#36 "Support M1 Macs using DisplayServicesGetBrigh…
cee27a8 
…tness/DisplayServicesSetBrightness".
@Mike-Woolley
Copy link

Thanks @jtbandes, this change works form me on both my M1 & Intel MacBooks 😄

However one line of the PR breaks brightness -l.
Reverting that line makes everything good - I have put a comment in the commit diff to indicate where.

Mike-Woolley
Mike-Woolley reviewed Feb 28, 2021
View reviewed changes
brightness.c
@@ -151,7 +232,7 @@ int main(int argc, char * const argv[]) {

float brightness;
if (action == ACTION_LIST) {
if (argc > 0) usage();
Copy link

@Mike-Woolley Mike-Woolley Feb 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This change breaks brightness -l.
Not sure what the purpose is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to fix an uninitialized variable warning, but I guess I made a mistake. Thanks for catching — I’ll submit a fix soon.

@nriley nriley self-assigned this Feb 28, 2021
@nriley nriley merged commit 09d5e57 into nriley:master Feb 28, 2021
@jtbandes
Copy link
Contributor Author

@nriley, you might want to revert the if (argc > 0) usage() change as recommended by @Mike-Woolley. I think when I was reading the code I didn't realize that argc was modified.

@nriley
Copy link
Owner

nriley commented Feb 28, 2021

Sorry, I did not mean to commit that as is. I've got a cleanup commit incoming.

@nriley
Copy link
Owner

nriley commented Feb 28, 2021

OK, updated and tested on all the Macs I have access to. Hopefully this works for everyone else too. Thanks again for the contribution!

@new3gum2
Copy link

new3gum2 commented Sep 17, 2022 •
edited
Loading

tested on M1 MacBook Pro 17,1.
Fails to work showing display id as 0x1 and error -536870201

@nriley
Copy link
Owner

nriley commented Sep 18, 2022

@new3gum2 are you building from source? If so, what macOS version and are you using the internal display?

@new3gum2
Copy link

@new3gum2 are you building from source? If so, what macOS version and are you using the internal display?
Sorry I didn't have time to test further, I used the make file and it worked like charm. it is macOS Monterey
I was trying the compiled versions from brew and MacPorts neither of those worked. But the source compile worked.

@KuSi833
Copy link

KuSi833 commented Oct 9, 2022

@new3gum2 are you building from source? If so, what macOS version and are you using the internal display?
Sorry I didn't have time to test further, I used the make file and it worked like charm. it is macOS Monterey
I was trying the compiled versions from brew and MacPorts neither of those worked. But the source compile worked.

Can also confirm building from source works, but was getting the same error when I installed it with brew.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mike-Woolley Mike-Woolley Mike-Woolley left review comments

Assignees

@nriley nriley

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Failing on Big Sur with Apple Silicon
5 participants
@jtbandes @nriley @Mike-Woolley @new3gum2 @KuSi833