Skip to content

Commit

Permalink
use reliable method calls for prop setters
Browse files Browse the repository at this point in the history
  • Loading branch information
Tony Crisci committed Nov 19, 2020
1 parent 890c224 commit c617911
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 14 deletions.
3 changes: 3 additions & 0 deletions playerctl/playerctl-cli.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ static gboolean playercmd_volume(PlayerctlPlayer *player, gchar **argv, gint arg
return FALSE;
}

g_debug("%s: setting volume to %f\n", instance, level);
playerctl_player_set_volume(player, level, &tmp_error);
if (tmp_error != NULL) {
g_propagate_error(error, tmp_error);
Expand Down Expand Up @@ -555,6 +556,7 @@ static gboolean playercmd_shuffle(PlayerctlPlayer *player, gchar **argv, gint ar
g_propagate_error(error, tmp_error);
return FALSE;
}
g_debug("%s: setting shuffle to %d\n", instance, status);
} else {
if (!pctl_player_has_cached_property(player, "Shuffle")) {
g_debug("%s: player has no shuffle status set, skipping", instance);
Expand Down Expand Up @@ -612,6 +614,7 @@ static gboolean playercmd_loop(PlayerctlPlayer *player, gchar **argv, gint argc,
return FALSE;
}

g_debug("%s: setting loop status to %d\n", instance, status);
playerctl_player_set_loop_status(player, status, &tmp_error);
if (tmp_error != NULL) {
g_propagate_error(error, tmp_error);
Expand Down
84 changes: 70 additions & 14 deletions playerctl/playerctl-player.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@

#define LENGTH(array) (sizeof array / sizeof array[0])

#define MPRIS_PATH "/org/mpris/MediaPlayer2"
#define PROPERTIES_IFACE "org.freedesktop.DBus.Properties"
#define PLAYER_IFACE "org.mpris.MediaPlayer2.Player"
#define SET_MEMBER "Set"

enum {
PROP_0,

Expand Down Expand Up @@ -81,6 +86,7 @@ struct _PlayerctlPlayerPrivate {
OrgMprisMediaPlayer2Player *proxy;
gchar *player_name;
gchar *instance;
gchar *bus_name;
PlayerctlSource source;
GError *init_error;
gboolean initted;
Expand Down Expand Up @@ -517,6 +523,7 @@ static void playerctl_player_finalize(GObject *gobject) {
g_free(self->priv->player_name);
g_free(self->priv->instance);
g_free(self->priv->cached_track_id);
g_free(self->priv->bus_name);

G_OBJECT_CLASS(playerctl_player_parent_class)->finalize(gobject);
}
Expand Down Expand Up @@ -950,6 +957,7 @@ static gboolean playerctl_player_initable_init(GInitable *initable, GCancellable
g_set_error(err, playerctl_player_error_quark(), 1, "Player not found");
return FALSE;
}
player->priv->bus_name = bus_name;

/* org.mpris.MediaPlayer2.{NAME}[.{INSTANCE}] */
int offset = strlen(MPRIS_PREFIX);
Expand All @@ -962,13 +970,10 @@ static gboolean playerctl_player_initable_init(GInitable *initable, GCancellable
pctl_source_to_bus_type(player->priv->source), G_DBUS_PROXY_FLAGS_NONE, bus_name,
"/org/mpris/MediaPlayer2", NULL, &tmp_error);
if (tmp_error != NULL) {
g_free(bus_name);
g_propagate_error(err, tmp_error);
return FALSE;
}

g_free(bus_name);

// init the cache
g_debug("initializing player: %s", player->priv->instance);
player->priv->cached_position =
Expand Down Expand Up @@ -1457,8 +1462,7 @@ gchar *playerctl_player_get_album(PlayerctlPlayer *self, GError **err) {
* maximum volume. Passing negative numbers should set the volume to 0.0.
*/
void playerctl_player_set_volume(PlayerctlPlayer *self, gdouble volume, GError **err) {
// TODO better error handling
// GError *tmp_error = NULL;
GError *tmp_error = NULL;

g_return_if_fail(self != NULL);
g_return_if_fail(err == NULL || *err == NULL);
Expand All @@ -1467,7 +1471,25 @@ void playerctl_player_set_volume(PlayerctlPlayer *self, gdouble volume, GError *
g_propagate_error(err, g_error_copy(self->priv->init_error));
return;
}
org_mpris_media_player2_player_set_volume(self->priv->proxy, volume);

GDBusConnection *connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &tmp_error);
if (tmp_error != NULL) {
g_propagate_error(err, tmp_error);
return;
}

GVariant *result = g_dbus_connection_call_sync(
connection, self->priv->bus_name, MPRIS_PATH, PROPERTIES_IFACE, SET_MEMBER,
g_variant_new("(ssv)", PLAYER_IFACE, "Volume", g_variant_new("d", volume)), NULL,
G_DBUS_CALL_FLAGS_NONE, -1, NULL, &tmp_error);
if (result != NULL) {
g_variant_unref(result);
}

if (tmp_error != NULL) {
g_propagate_error(err, tmp_error);
return;
}
}

/**
Expand All @@ -1489,10 +1511,10 @@ gint64 playerctl_player_get_position(PlayerctlPlayer *self, GError **err) {
return 0;
}

GVariant *call_reply = g_dbus_proxy_call_sync(
G_DBUS_PROXY(self->priv->proxy), "org.freedesktop.DBus.Properties.Get",
g_variant_new("(ss)", "org.mpris.MediaPlayer2.Player", "Position"), G_DBUS_CALL_FLAGS_NONE,
-1, NULL, &tmp_error);
GVariant *call_reply = g_dbus_proxy_call_sync(G_DBUS_PROXY(self->priv->proxy),
"org.freedesktop.DBus.Properties.Get",
g_variant_new("(ss)", PLAYER_IFACE, "Position"),
G_DBUS_CALL_FLAGS_NONE, -1, NULL, &tmp_error);
if (tmp_error) {
g_propagate_error(err, tmp_error);
return 0;
Expand Down Expand Up @@ -1563,6 +1585,7 @@ void playerctl_player_set_position(PlayerctlPlayer *self, gint64 position, GErro
*/
void playerctl_player_set_loop_status(PlayerctlPlayer *self, PlayerctlLoopStatus status,
GError **err) {
GError *tmp_error = NULL;
g_return_if_fail(self != NULL);
g_return_if_fail(err == NULL || *err == NULL);

Expand All @@ -1574,8 +1597,24 @@ void playerctl_player_set_loop_status(PlayerctlPlayer *self, PlayerctlLoopStatus
const gchar *status_str = pctl_loop_status_to_string(status);
g_return_if_fail(status_str != NULL);

// TODO better error handling
org_mpris_media_player2_player_set_loop_status(self->priv->proxy, status_str);
GDBusConnection *connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &tmp_error);
if (tmp_error != NULL) {
g_propagate_error(err, tmp_error);
return;
}

GVariant *result = g_dbus_connection_call_sync(
connection, self->priv->bus_name, MPRIS_PATH, PROPERTIES_IFACE, SET_MEMBER,
g_variant_new("(ssv)", PLAYER_IFACE, "LoopStatus", g_variant_new("s", status_str)), NULL,
G_DBUS_CALL_FLAGS_NONE, -1, NULL, &tmp_error);
if (result != NULL) {
g_variant_unref(result);
}

if (tmp_error != NULL) {
g_propagate_error(err, tmp_error);
return;
}
}

/**
Expand All @@ -1587,6 +1626,7 @@ void playerctl_player_set_loop_status(PlayerctlPlayer *self, PlayerctlLoopStatus
* Request to set the shuffle state of the player, either on or off.
*/
void playerctl_player_set_shuffle(PlayerctlPlayer *self, gboolean shuffle, GError **err) {
GError *tmp_error = NULL;
g_return_if_fail(self != NULL);
g_return_if_fail(err == NULL || *err == NULL);

Expand All @@ -1595,8 +1635,24 @@ void playerctl_player_set_shuffle(PlayerctlPlayer *self, gboolean shuffle, GErro
return;
}

// TODO better error handling
org_mpris_media_player2_player_set_shuffle(self->priv->proxy, shuffle);
GDBusConnection *connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &tmp_error);
if (tmp_error != NULL) {
g_propagate_error(err, tmp_error);
return;
}

GVariant *result = g_dbus_connection_call_sync(
connection, self->priv->bus_name, MPRIS_PATH, PROPERTIES_IFACE, SET_MEMBER,
g_variant_new("(ssv)", PLAYER_IFACE, "Shuffle", g_variant_new("b", shuffle)), NULL,
G_DBUS_CALL_FLAGS_NONE, -1, NULL, &tmp_error);
if (result != NULL) {
g_variant_unref(result);
}

if (tmp_error != NULL) {
g_propagate_error(err, tmp_error);
return;
}
}

char *pctl_player_get_instance(PlayerctlPlayer *player) {
Expand Down
3 changes: 3 additions & 0 deletions test/test_commands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ def get_called(cmd):
results = await asyncio.gather(*(playerctl.run(f'-p commands {cmd}')
for cmd in commands + setters))

for result in results:
assert result.returncode == 0, result.stderr

for i, cmd in enumerate(commands):
result = results[i]
assert get_called(cmd), f'{cmd} was not called: {result.stderr}'
Expand Down

0 comments on commit c617911

Please sign in to comment.