Remove the feedback and the associated callback data before invoking the
callback (which then triggers FbdFeebackBased:ended) since we otherwise
remove the data from the currently playing sound from the hash table
instead of removing the data from the sound that just finished playing.
Call flow for a repeating sound before:
on_sound_play_finished_callback()
data->callback()
fbd_feedback_get_ended()
on_fb_ended()
fbd_feedback_run()
g_hash_table_insert(..., feedback, data)
fbd_dev_sound_play()
gsound_context_play_full() [async]
g_hash_table_remove (..., data->feedback) <- removes wrong feedback
So (since the feedback is still the same but only the callback data
changes) we end up removing the data from the currently playing sound
from the hash table and any calls to fbd_dev_sound_stop() would fail
to find data and hence not cancel the playing sound.
Call flow for a repeating sound now:
on_sound_play_finished_callback()
g_hash_table_remove (..., data->feedback)
data->callback()
fbd_feedback_get_ended()
on_fb_ended()
fbd_feedback_run()
g_hash_table_insert(..., feedback, data)
fbd_dev_sound_play()
gsound_context_play_full() [async]
So we remove the data of the currently playing sound before adding
the feedback to the hash table again. No ownership is transfered to
the hash table so no cleanusp need to be changed.
Closes: https://source.puri.sm/Librem5/feedbackd/-/issues/46
Phosh recently changed to XDG_CURRENT_DESKTOP=GNOME:Phosh and has since stopped
using the configured theme and instead used the fallback freedesktop theme.
Closes#41
This would allow users to create their own feedback profiles
without editing system files or using root privileges
Signed-off-by: Pablo Correa Gómez <ablocorrea@hotmail.com>
The README currently states that feedbackd uses the `default.json` theme, whose
location is hardcoded into the application. This doesn't reflect the current
state of the code.
Explain the ability to add custom themes and give some examples to the user.
Also mention the `feedbackd-device-themes` repository explicitly for users
looking to modify the current `default.json` theme. Adds explanations about the
correct naming of custom theme files and theme file precedence when multiple
themes exist.
Previously, assembly of the theme paths for custom paths (based on
`XDG_DATA_DIRS`) was performed using `g_strconcat`, which produced erroneous
file paths with missing path separators due to a typo in the code.
Instead, make use of `g_build_filename` to assemble the theme paths from
`XDG_DATA_DIRS` to look for custom themes, which automatically introduces
host-specific path separators between individual path components. This
introduces another temporary variable to ensure that the filename (which is
itself assembled from a name and the file ending) isn't mangled by introduction
of a path separator between these two "parts".
The current logic was fragile and failed e.g. when the
FEEDBACKD_UDEV_VAL_LED would not match leaking the dev since the
g_object_unref() at the end of the loop was never hit.
Prevent that with automatic cleanup and explicitly ref'ing the
devs we want to use.
When the client goes away we build up a list of events to end feedback
for to make sure the hash table isn't modified in place. tThe alternative
approach would be to disconnect the on_event_feedbacks_ended handler,
duplicate the DBus signal emission and use g_hash_table_foreach_remove
but that looks less robust than using the same code path.
Closes: #25
If the trigger is the last thing in sysfs_path/LED_TRIGGER_ATTR, then
the new-line at the end of the output causes g_strv_contains to fail to
find the trigger in the list. This strips the new-line before splitting
the output into a list for searching.
You can see the problem this patch fixes with the librem 5, by:
$ rmmod ledtrig-timer # or whatever is listed at the end, so 'pattern' is the last thing
$ cat /sys/devices/platform/pwmleds/leds/blue:status/trigger
[none] kbd-scrolllock ... pattern
$ fbd-ledctrl -p /sys/devices/platform/pwmleds/leds/blue:status -t pattern
Failed to set perms of /sys/devices/platform/pwmleds/leds/blue:status/pattern to 101: No such file or directory
$ modprobe ledtrig-timer
$ cat /sys/devices/platform/pwmleds/leds/blue:status/trigger
[none] kbd-scrolllock ... pattern timer
$ fbd-ledctrl -p /sys/devices/platform/pwmleds/leds/blue:status -t pattern
<success>
Evangelos Ribeiro Tzaras
Pablo Correa Gómez
Guido Günther
Andreas Hartmann
Evangelos Ribeiro Tzaras
Sebastian Spaeth
Clayton Craft
Martin Bürgmann