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 hueshift block & updated list of blocks #802

Merged
merged 15 commits into from
Sep 3, 2020

Conversation

AkechiShiro
Copy link
Contributor

  • Scrolling up or down updates the screen temperature by a "step"
    amount (max_step 2000K).
  • Supports either Xsct (X11 set screen color) or Redshift to change screen color temperature.
  • Redshift is preferred as it as a much larger range of temperature
    color changes (between 1000K and 25_000K).
  • Currently for testing purpose only redshift is supported.

I'm open to any feedback or suggestion.
I'm a fairly new to Rust and willing to learn if there is anything I can make a little nicer, just say so.
Also I've used u16 because the temperature of the color should never be negative nor go over 65535 since the highest limit I've seen is 25 000K that Redshift can set.

I'm also planning to add a MouseButton event later perhaps to toogle between different values instantly by clicking on the text.
But first I have to find out what's going on with the JSON error #801 I'm getting from i3bar.

* Scrolling up or down updates the screen temperature by a "step"
  amount (max_step 2000K).
* Supports either sct or redshift to change screen color temperature.
* Redshift is preferred as it as a much larger range of temperature
  color changes (between 1000K and 25_000K).
* Currently for testing purpose only redshift is supported.
* Check command with has_command from util.rs
* Redshift stdout/stderr redirected to /dev/null
* New JSON error appears when the block is scrolled
@omertuc
Copy link
Contributor

omertuc commented Aug 16, 2020

You forgot to update the documentation to include your block and its configuration options

* Update documentation with parameters and block description
* Added checks in code (whether using sct, redshift)
src/blocks/hueshift.rs Outdated Show resolved Hide resolved
@ammgws
Copy link
Collaborator

ammgws commented Aug 25, 2020

Does your output still look like this when scrolling?

{"version": 1, "click_events": true}
[[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6500 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],
[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6500 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],
5
[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6500 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],
5
[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6500 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],
5
[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6500 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],
5
[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6500 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],
[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 6100 ","markup":"pango","name":"8e354b15488f46a9bbbf97e89ccd7574","separator":false,"separator_block_width":0}],

As a sanity check can you change your scroll commands to "echo test" and see what the output looks like?

@AkechiShiro
Copy link
Contributor Author

AkechiShiro commented Aug 25, 2020

Yeah it still does look like that when scrolling down.
I did what you told me, and here's is what I'm getting now, we're making a little bit of progress understanding a bit more the issue here, I guess.
It seems the 5 or the 4 are related to whether I scroll up or down.
The command I wrote inside the code is the following : echo test >/tmp/debugi3rs 2>&1

[{"background":"#000000","color":"#a9a9a9","full_text":"| ","markup":"pango","separator":false,"separator_block_width":0},{"background":"#000000","color":"#93a1a1","full_text":" 5700 ","markup":"pango","name":"90ac1a68fe8d44578291852aad88f6a9","separator":false,"separator_block_width":0}],
tail: /tmp/debugi3rs: file truncated
test
4

When I scroll up a 4 get printed, when I scroll down a 5 shows up, finally when I click onto the block a 1 shows up and the JSON error shows up as well.

@AkechiShiro
Copy link
Contributor Author

I will also add that a 3 is printed when I right-click on it and a 2 is printed when I use the mouse3 button on it.
To me this seems like some kind of debug for the mouse events that is currently getting printed.

@AkechiShiro
Copy link
Contributor Author

I've got even more interesting now, even without the new block the JSON error appears with the numbers as well in the debug.
I just need to scroll anywhere on the status bar.
This bug seems to happen (I'm running i3-gaps, and the status bar having this error is the bottom one, I have an upper one too on which scrolling works).
I've tested the hueshift block on the upper one and the same JSON error does not appear at all.
On the bottom status bar there is also the i3 workspace numbering 1 to 10 showing and if depending on the direction on which I'm scrolling I can switch to other workspaces.

So i3 seems to be somewhat interfering with the i3status-rs JSON output I suppose, how that is happening is a little beyond me to be honest but the link is that scrolling does two actions on the bottom status bar.

@ammgws
Copy link
Collaborator

ammgws commented Aug 26, 2020

What's the bare minimum config you can reproduce this with?

@AkechiShiro
Copy link
Contributor Author

This is the bare minimum config I can reproduce this with :

[[block]]
block = "hueshift"

Or with any other config that doesn't have this block either, it happens even if I choose another block than the custom one I've made.
However this issue seems to be intermittent, right now when I scroll on the i3 bottom status bar, the JSON error does not get triggered anymore, this could be harder to reproduce since it doesn't seem to be related to the i3status-rs config file at all.

@ammgws
Copy link
Collaborator

ammgws commented Aug 26, 2020

OK I can reproduce the numbers in the output, however they are only logged to stderr so it shouldn't have an affect on i3/swaybar. When using tee command from before it was redirecting stderr to stdout which explains why it was causing those JSON errors.

I think the stderr output was added by accident in fe517af. I'm thinking about added a --debug flag since it has been mentioned before, but until then I'll just remove it for now.

@AkechiShiro
Copy link
Contributor Author

I can confirm that without the debug the sliding works with no errors on the bottom status bar. Seems the redirection of the stderr in stdout was indeed the culprit.
Right now, there is just one more issue I'm having, it's that when I scroll the color temperature does change. But then when an update of the bar happens the screen goes back to the default color temperature (6500K) instead of the number that is shown updated in the bar.
Any idea why that is happening ?

* Add redshift -P flag to reset existing gamma ramps before applying new
  color effect.
@AkechiShiro
Copy link
Contributor Author

The block is done and it's documentation is too, I did not test however changing the temperature when setting the hue_shifter to sct but with redshift the block seems to work as intended.

My issue came from a custom block I had made to change my color temperature every time the i3bar updated the command with the default 6500K would change back conflicting with the new hueshift block.
There are no more issue I believe, if any changes are required or any more documentation needed feel free to ask me and I'll make the required changes.
This PR I believe is now ready to be merged with master.
Thanks for all the help given here.

* Fix a "bug" if max_temp and min_temp are a range in which current_temp is not.
* Fix a conditional mistake about ensuring that min_temp gets ensured
  to not go under 1000K.
* Fix step enforcing of a "discrete" step limit.
* Move assignement of new_temp outside of match to avoid writing two
  assignements in each matched members.
* Details some more the documentation of the block
* Add right click to reset hue color temperature to default (6500K)
Copy link
Contributor

@omertuc omertuc left a comment

Choose a reason for hiding this comment

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

Works great otherwise, thanks for this block, it's super useful for me!

src/blocks/hueshift.rs Outdated Show resolved Hide resolved
src/blocks/hueshift.rs Outdated Show resolved Hide resolved
blocks.md Outdated Show resolved Hide resolved
@omertuc
Copy link
Contributor

omertuc commented Aug 31, 2020

A bug I found while using it throughout the day: Seems like left click anywhere in the bar (for example, pausing/unpausing music) triggers the left click action on the hueshift block

@omertuc
Copy link
Contributor

omertuc commented Aug 31, 2020

A bug I found while using it throughout the day: Seems like left click anywhere in the bar (for example, pausing/unpausing music) triggers the left click action on the hueshift block

Figured out what causes it, you need to add a check that the click event was on your block ID, like this:
image

* Match the blog by its id when an event is triggered
* Left click changed to now switch to set color temperature in
  click_temp variable (default to 6500K)
@AkechiShiro
Copy link
Contributor Author

Thanks for the help to fix the bug, I believe that the block is now fully ready, if you wish to test it some more and provide some feedback however I think there won't be any issues anymore.
I've also updated the doc describing the left click behavior and its config argument click_temp.
Maybe a neat upgrade would be to manage some kind of state, and when the left_click is pressed once the state switch and the click_temp value is set then if another left_click is triggered a reset to a default_temp happens, but currently it is possible to do this by doing a left click then a right click so I'm not too sure it's really necessary.

@ammgws
Copy link
Collaborator

ammgws commented Sep 3, 2020

LGTM and looks like you already have a happy user in @omertuc

I checked some other status bars and apart from geolocation/GPS (which I don't think should be handled by us), it looks like we are feature complete.
https://bumblebee-status.readthedocs.io/en/latest/modules.html?highlight=redshift#redshift
https://py3status.readthedocs.io/en/latest/modules.html#hueshift

Thanks!

@ammgws ammgws merged commit a211461 into greshake:master Sep 3, 2020
@AkechiShiro
Copy link
Contributor Author

AkechiShiro commented Nov 13, 2020

Also, I'm thinking of opening another PR or continuing this one if possible?

To improve the hueshift block.

The current implementation does not work under Wayland (which I was going to transition to), even if i3status-rust works on Wayland.

Redshift has a specific package which works well under Wayland provided you use the -m wayland method.
Unfortunately, there is no easy way to distinguish the redshift that is patched to work with Wayland and the one that isn't (the one that isn't just spits an error when given the -m wayland option).

However, I'm thinking that to be broader, I should try to include more Wayland compatible hue shifting programs to the hueshift block such as gammastep.

@aloispichler
Copy link

aloispichler commented Nov 7, 2022

Thanks for this widget. It recognizes the hue_shifter automatically (redshift, on my machine) and changes the temperature upon scrolling.
But the shifter does not autostart, and it does not change the temperature displayed, if it changes during the day. I would expect the displayed temperature to change at the beginning of the night (and day). Further, an icon is missing. Is it a problem on my side?

@MaxVerevkin
Copy link
Collaborator

and it does not change the temperature displayed, if it changes during the day

I might be wrong, but AFAIK there is no way to query current temperature from redshift. This is not a limitation of this block because it is able to monitor changes when wl_gammarelay(_rs) is used.

But the shifter does not autostart

Can you describe current behavior and expected behavior?

Further, an icon is missing

Covered in #1465

@naveg
Copy link

naveg commented Nov 16, 2022

I might be wrong, but AFAIK there is no way to query current temperature from redshift.

I believe redshift -p does this.

I agree that it would be a good improvement for the block to display the current temperature, as set by redshift itself outside of the status bar.

@MaxVerevkin
Copy link
Collaborator

I believe redshift -p does this.

Good to know! I did some initial work in 34e2b45, PRs implementing temperature updates for concrete back-ends are welcome.

@tusharxoxoxo tusharxoxoxo mentioned this pull request Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants