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

Cannot dump template configuration file #1

Open
fmkang opened this issue Jun 4, 2020 · 4 comments
Open

Cannot dump template configuration file #1

fmkang opened this issue Jun 4, 2020 · 4 comments

Comments

@fmkang
Copy link

fmkang commented Jun 4, 2020

Hi, I tried to start with the template configuration in your readme. Unfortunately, it won't accept a JSON with comments and failed to start, despite that brew still showed "Successfully started brisyncd". So I tried brisyncd config > /usr/local/etc/brisyncd.json to dump one from the tool, but it shows "command not found: brisyncd". I guess it's because it's not added to PATH. Then I used /usr/local/opt/brisyncd/sbin/brisyncd config > /usr/local/etc/brisyncd.json and it outputs "Error: Failed to parse config file [/usr/local/etc/brisyncd.json]". The tool seems to parse it before writing it. I have to let the tool print it to stdout and copy it to the JSON file by hand.

Also, I think you should describe more about the configuration file's format. I read the help information and I don't know if --min and --max are the same as "min" and "max" under "source" or "targets" in the configuration file. And I don't know how "min" and "max" of the source are mapped to the "min" and "max" of the target. I thought source.min -> target.min and source.max -> target.max, but it seems not. Currently I just leave out those under "source" and only use the target ones, assuming it means 0% on the target (external display) looks like target.min% on the source (built-in display) and 100% on the target (external display) looks like target.max% on the source. Did I interpret it right?

@aleksey-mashanov
Copy link
Owner

Thank you for your feedback!

Example configuration in README includes comments for readability but JSON itself does not support them. Of course they should be removed on copy-paste. Of course this must be explained in README, I'll fix it.

If installed using brew brisyncd is symlinked to /usr/local/sbin/brisyncd. This place is more canonical for daemons which is not primarily supposed to be run by user. It looks like in macOS /usr/local/sbin is not in $PATH by default. I will mention this in README.

Yes, brisyncd reads existing configuration before dumping a new. It will include all known monitors to this dump, not only currently connected. This is done intentionally to simplify new monitors configuration (for example, if you have one in the home and another in the office).
Unfortunately command mentioned in README is wrong. Shell empties the file before brisyncd starts. I will add -o filename option to solve this race.

Only one parameters is about source display - "source". All other parameters are about targets. Defaults are at the top level - they can be used for any unknown monitors if you often use them (for example in conference rooms). For personal displays, I think, defaults are useless because it is better to tune displays individually. And yes, you do it right.

@fmkang
Copy link
Author

fmkang commented Jun 7, 2020

I see. This answered my question. I think you can even create some way to let users collaborate on settings for different monitors. And it would be better if the tool can be temporarily disabled. For example, when the lid is half-closed, the sensor's reading becomes low but you don't want to dim your screen.

The tool works perfectly when the brightness is adjusted automatically, but it's not very smooth or even not responding when I manually adjust the brightness. Using a shorter interval didn't help. I also noticed you will lower the contrast when the brightness becomes lower than the target display's lowest brightness. However, the contrast is not restored when the brightness goes above zero again, and thus the display looks too dark.

Thank you for bringing such a brilliant project.

@aleksey-mashanov
Copy link
Owner

I want brisyncd code to be simple for security/privacy review, so I don't want it to have any heavy features such as UI or keyboard shortcuts. Something like brisyncctl is too complex for such a simple tool too. The best way to temporarily disable brisyncd is to stop it using brew services stop brisyncd.
Nonetheless I have plans to add a rollback to contrast/brightness values a second before the lid was closed to mitigate display darkening because of the lid closing.

Your monitor can silently ignore DDC/CI commands if they appear too frequently with too short interval between them. By DDC/CI specification minimum interval between contrast/brightness modification commands is 50ms. (this is the default value for "interval" option in brisyncd config). Therefore lowering interval bellow this value is not recommended. If your monitor still skips commands with 50ms. then increase it.
I had the same problem with Dell U2720Q. Brisyncd ver. 1.1.0 with 50ms. interval fixed it for me.

You can see debug logs using

log stream --process brisyncd --level debug

In the log commands sent to a display looks like

brisyncd: Target display [DELL U2720Q] contrast set to 75%
brisyncd: Target display [DELL U2720Q] brightness set to 2%

@aleksey-mashanov
Copy link
Owner

I tried to start with the template configuration in your readme. Unfortunately, it won't accept a JSON with comments and failed to start.

Added a note in README.md.

So I tried brisyncd config > /usr/local/etc/brisyncd.json to dump one from the tool, but it shows "command not found: brisyncd".

The command is prefixed with /usr/local/sbin/ in README.md.

Then I used /usr/local/opt/brisyncd/sbin/brisyncd config > /usr/local/etc/brisyncd.json and it outputs "Error: Failed to parse config file [/usr/local/etc/brisyncd.json]".

Added -o <filename> option to brisyncd config subcommand to use instead of stdout redirection (>).

I also noticed you will lower the contrast when the brightness becomes lower than the target display's lowest brightness. However, the contrast is not restored when the brightness goes above zero again, and thus the display looks too dark.

At some point of time I started to experience the same problem. I tried to solve this by replacing a library used to communicate with a display with my own implementation. Hope this will help. I used it for about 2 weeks without any visible glitches.

You can update brisyncd to version 1.2.0 to try these changes.

P.S. And this one still waits in the backlog:

Nonetheless I have plans to add a rollback to contrast/brightness values a second before the lid was closed to mitigate display darkening because of the lid closing.

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

No branches or pull requests

2 participants