`wpaperd` crashes when `group` option is set

[AntoineGuilminCrepon]

So, I wanted to try the new group option, to synchronize the wallpapers of my two displays. Before someone asks, yes, I am compiling from the latest git version, I realized the option is not available in the last stable release. However, I got a very unexpected result: wpaperd just crashes, while telling that it doesn't recognize the group field in the config file.

After looking through the code, it seems that the config.rs file doesn't parse the group option, which looks like an oversight ? Or maybe I missed something, I'm not sure...

I tried to wrap my head around the way that WallpaperGroup interact with Wpaperd objects, to try to fix the issue and propose a patch, but I don't quite get it, sadly.

[jervw]

I am also unable to get the group option working properly using master branch. It would be nice to get some additional documentation/examples on how it works.

[danyspin97]

I can confirm that group is not working as expected. I'll take a look at soon as possible.

After looking through the code, it seems that the config.rs file doesn't parse the group option, which looks like an oversight ?

The group is set inside sorting, as GroupedRandom { group: u8 }, while the documentation refers to group, this is why config.rs is not handling it the way the documentation explains it.

[coffee243454]

After a little bit of testing I was able to get the group option working for me the way I wanted.

Here's the config I used:

[any] path = WALLPAPER_PATH [default.sorting.groupedrandom] group = 1

With this I get the same random wallpaper on all screens.

[MauroGuida]

I'm not very experienced with Rust, so there may be a better way to do this, but by adding:

(Some(Sorting::Random), _) | (_, Some(Sorting::Random)) if self.group.is_some() => Some(Sorting::GroupedRandom { group: self.group.unwrap() }),

after line 134 in config.rs resolves the issue.

The code for let sorting at line 133 (config.rs) should look like this:

let sorting = match (&self.sorting, &default.sorting) {
    (None, Some(_)) if path.is_file() && !path_inherited => None,
    (Some(Sorting::Random), _) | (_, Some(Sorting::Random)) if self.group.is_some() => Some(Sorting::GroupedRandom { group: self.group.unwrap() }),
    (Some(sorting), _) | (None, Some(sorting)) => Some(*sorting),
    (None, None) => None,
};

EDIT: I forgot to mention that you also need to add pub group: Option<u8> below line 58 in config.rs

[danyspin97]

Hello everybody, sorry for the late reply by life has been hectic lately. I am back with a new commit fixing this issue.

Now you should be able to write:

[default]
path = "..." 
sorting = "random" 
group = 1

Thanks everybody for testing an providing feedback. Big thanks also to @MauroGuida for providing the base for the fix, I espanded your idea and added a couple of checks and covered edge cases.

I am also planning to expand group to work with ascending and descending sorting, as well as omitting the path when not needed. However, that's for later.

Can you please try master and check?

[MauroGuida]

Replying to #89 (comment)

With the following configuration, random grouping in the master branch doesn't work for me:

[default]
path = "/mnt/Dati/Pictures/Wallpapers/Wallpapers_1"
duration = "5m"
mode = "center"
sorting = "random"
queue-size = 100

[DP-1]
offset = 1.0
group = 1

[DP-2]
offset = 0.0
group = 1

However, grouping works if sorting is not specified. I believe this issue is related to a check in config.rs at line 198:

let sorting = if group.is_some() && sorting.is_none() {
    // Assign it the default one, so that we can do the group match below
    Some(Sorting::default().into())
} else {
    None
};

Without the second check sorting.is_none() everything seems to work fine.

EDIT: With the previously mentioned sorting assignment in line 198, ascending and descending don't seem to work as expected. Without it (thus removing the sorting assignment), everything seems to work: ascending, descending, random, and random grouping.

[danyspin97]

However, grouping works if sorting is not specified. I believe this issue is related to a check in config.rs at line 198:

Yea, that is exactly the issue. By returning None I was discarding the previous value, while it should have been sorting directly. Pushed a new commit.

[MauroGuida]

However, grouping works if sorting is not specified. I believe this issue is related to a check in config.rs at line 198:

Yea, that is exactly the issue. By returning None I was discarding the previous value, while it should have been sorting directly. Pushed a new commit.

It now works as intended with every sorting mode. Thank you!

[danyspin97]

It now works as intended with every sorting mode. Thank you!

Glad to hear that :) Closing the issue then. Please feel free to reopen or open a new one for any issue.