Introduce new profile-oriented API

Almost all of the devices we've worked with here are the same: they have
some sort of profile on them, and if they don't (which is quite rare)
it's rather easy to simulate one in the driver. In fact basically all of
the device drivers in ratbag just update the profile and rewrite it on
calls to the write_resolution_dpi() and write_button() calls. This is
annoying since profile writes can take a good bit of time, and we're
wasting a lot of time with redundant profile writes that way.

During XDC this year, we came up with an idea that makes more sense: a
profile oriented API. Instead of having multiple entrypoints in the API
to writing and updating the profile we just cache all of the profile
changes until ratbag_device_commit_profiles() is called, at which point
we do a single set of writes for each profile and button that changed.

This has a lot of advantages:
- Where the profile is and is not written out to the device is now
  explicit.
- Changing attributes of each profile won't result in any sort of delay
  up until the library user commits the profiles, allowing us to avoid
  hanging the application every time a simple attribute is changed.
- We can cut down on workarounds in drivers to cater to the previous
  API.

Since this commit doesn't handle converting all of the device drivers
over to the new API, we also add a temporary wrapper for devices that
haven't yet been converted to this new API so that they're still usable.
Once all of the drivers have been converted to the new API, the wrapper
will be removed along with the internal driver API calls.

Signed-off-by: Lyude <thatslyude@gmail.com>

Changes since v1:
- has_changed→dirty
- Change namespace to LIBRATBAG_0.5.0
Changes since v2:
- Remove unneeded note about not setting dirty bit
- commit_profiles->commit
- Make FIXMEs more obvious
- Minor styling nitpicks
- Split up capability check fixes into seperate patch

src/libratbag-private.h

@@ -130,6 +130,18 @@ struct ratbag_driver {
 	 */
 	void (*remove)(struct ratbag_device *device);
 
+	/**
+	 * Callback called when the driver should write any profiles that
+	 * were modified back to the device.
+	 *
+	 * Both profile and button structs have a dirty variable that can
+	 * be used to tell whether or not they've actually changed since
+	 * the last commit. In order to reduce the amount of time
+	 * committing takes, drivers should use this information to avoid
+	 * writing back profiles and buttons that haven't actually changed.
+	 */
+	int (*commit)(struct ratbag_device *device);
+
 	/**
 	 * Callback called when a read profile is requested by the
 	 * caller of the library.
@@ -141,9 +153,10 @@ struct ratbag_driver {
 	 */
 	void (*read_profile)(struct ratbag_profile *profile, unsigned int index);
 
-	/**
-	 * Here, the driver should actually write the profile to the
-	 * device.
+	/*
+	 * FIXME: This function is deprecated and should not be removed. Once
+	 * we've updated all the device drivers to stop using it we'll remove
+	 * it. Look at commit() instead.
 	 */
 	int (*write_profile)(struct ratbag_profile *profile);
 
@@ -168,18 +181,18 @@ struct ratbag_driver {
 	 */
 	void (*read_button)(struct ratbag_button *button);
 
-	/**
-	 * For the given button, store in the profile and in the device
-	 * the given struct ratbag_button.
+	/*
+	 * FIXME: This function is deprecated and should not be removed. Once
+	 * we've updated all the device drivers to stop using it we'll remove
+	 * it. Look at commit() instead.
 	 */
 	int (*write_button)(struct ratbag_button *button,
 			    const struct ratbag_button_action *action);
 
-	/**
-	 * For the given profile, overwrite the current resolution
-	 * of the sensor expressed in DPI, and commit it to the hardware.
-	 *
-	 * Mandatory if the driver exports RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION.
+	/*
+	 * FIXME: This function is deprecated and should not be removed. Once
+	 * we've updated all the device drivers to stop using it we'll remove
+	 * it. Look at commit() instead.
 	 */
 	int (*write_resolution_dpi)(struct ratbag_resolution *resolution,
 				    int dpi_x, int dpi_y);
@@ -218,6 +231,7 @@ struct ratbag_profile {
 	} resolution;
 
 	bool is_active;		/**< profile is the currently active one */
+	bool dirty;       /**< profile changed since last commit */
 };
 
 #define BUTTON_ACTION_NONE \
@@ -277,6 +291,7 @@ struct ratbag_button {
 	enum ratbag_button_type type;
 	struct ratbag_button_action action;
 	uint32_t action_caps;
+	bool dirty; /* changed since last commit to device */
 };
 
 static inline void

src/libratbag.c

@@ -749,6 +749,87 @@ ratbag_device_has_capability(const struct ratbag_device *device,
 	return !!(device->capabilities & (1UL << cap));
 }
 
+/* FIXME: This is a temporary fix for all of the drivers that have yet to be
+ * converted to the new profile-oriented API. Once all of the drivers have been
+ * converted, this code should be removed.
+ */
+static enum ratbag_error_code
+ratbag_old_write_profile(struct ratbag_device *device)
+{
+	struct ratbag_profile *profile;
+	struct ratbag_button *button;
+	struct ratbag_resolution *resolution;
+	int rc;
+	unsigned int i;
+
+	assert(device->driver->write_profile);
+
+	list_for_each(profile, &device->profiles, link) {
+		if (!profile->dirty)
+			continue;
+
+		rc = device->driver->write_profile(profile);
+		if (rc)
+			return RATBAG_ERROR_DEVICE;
+
+		if (device->driver->write_resolution_dpi) {
+			for (i = 0; i < profile->resolution.num_modes; i++) {
+				resolution = &profile->resolution.modes[i];
+
+				rc = device->driver->write_resolution_dpi(
+				    resolution, resolution->dpi_x,
+				    resolution->dpi_y);
+				if (rc)
+					return RATBAG_ERROR_DEVICE;
+			}
+		}
+
+		if (device->driver->write_button) {
+			list_for_each(button, &profile->buttons, link) {
+				struct ratbag_button_action action =
+					button->action;
+
+				if (!button->dirty)
+					continue;
+
+				rc = device->driver->write_button(button,
+								  &action);
+				if (rc)
+					return RATBAG_ERROR_DEVICE;
+			}
+		}
+	}
+
+	return RATBAG_SUCCESS;
+}
+
+LIBRATBAG_EXPORT enum ratbag_error_code
+ratbag_device_commit(struct ratbag_device *device)
+{
+	struct ratbag_profile *profile;
+	struct ratbag_button *button;
+	int rc;
+
+	if (!device->driver->commit) {
+		rc = ratbag_old_write_profile(device);
+		if (rc != RATBAG_SUCCESS)
+			return rc;
+	} else {
+		rc = device->driver->commit(device);
+		if (rc)
+			return RATBAG_ERROR_DEVICE;
+	}
+
+	list_for_each(profile, &device->profiles, link) {
+		profile->dirty = false;
+
+		list_for_each(button, &profile->buttons, link)
+			button->dirty = false;
+	}
+
+	return RATBAG_SUCCESS;
+}
+
 LIBRATBAG_EXPORT enum ratbag_error_code
 ratbag_profile_set_active(struct ratbag_profile *profile)
 {
@@ -759,11 +840,6 @@ ratbag_profile_set_active(struct ratbag_profile *profile)
 	if (!ratbag_device_has_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE))
 		return RATBAG_ERROR_CAPABILITY;
 
-	assert(device->driver->write_profile);
-	rc = device->driver->write_profile(profile);
-	if (rc)
-		return RATBAG_ERROR_DEVICE;
-
 	assert(device->driver->set_active_profile);
 	rc = device->driver->set_active_profile(device, profile->index);
 	if (rc)
@@ -843,15 +919,12 @@ ratbag_resolution_set_dpi(struct ratbag_resolution *resolution,
 			  unsigned int dpi)
 {
 	struct ratbag_profile *profile = resolution->profile;
-	int rc;
 
 	resolution->dpi_x = dpi;
 	resolution->dpi_y = dpi;
+	profile->dirty = true;
 
-	assert(profile->device->driver->write_resolution_dpi);
-	rc = profile->device->driver->write_resolution_dpi(resolution, dpi, dpi);
-
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT enum ratbag_error_code
@@ -859,7 +932,6 @@ ratbag_resolution_set_dpi_xy(struct ratbag_resolution *resolution,
 			     unsigned int x, unsigned int y)
 {
 	struct ratbag_profile *profile = resolution->profile;
-	int rc;
 
 	if (!ratbag_resolution_has_capability(resolution,
 					      RATBAG_RESOLUTION_CAP_SEPARATE_XY_RESOLUTION))
@@ -868,18 +940,21 @@ ratbag_resolution_set_dpi_xy(struct ratbag_resolution *resolution,
 	if ((x == 0 && y != 0) || (x != 0 && y == 0))
 		return RATBAG_ERROR_VALUE;
 
-	assert(profile->device->driver->write_resolution_dpi);
-	rc = profile->device->driver->write_resolution_dpi(resolution, x, y);
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	resolution->dpi_x = x;
+	resolution->dpi_y = y;
+	profile->dirty = true;
+
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT enum ratbag_error_code
 ratbag_resolution_set_report_rate(struct ratbag_resolution *resolution,
 				  unsigned int hz)
 {
+	resolution->profile->dirty = true;
 	resolution->hz = hz;
-	/* FIXME: call into the driver */
-	return RATBAG_ERROR_IMPLEMENTATION;
+
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT int
@@ -930,9 +1005,10 @@ ratbag_resolution_is_default(const struct ratbag_resolution *resolution)
 LIBRATBAG_EXPORT enum ratbag_error_code
 ratbag_resolution_set_default(struct ratbag_resolution *resolution)
 {
+	resolution->profile->dirty = true;
 	resolution->is_default = true;
-	/* FIXME: call into the driver */
-	return RATBAG_ERROR_IMPLEMENTATION;
+
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT struct ratbag_button*
@@ -1000,7 +1076,6 @@ LIBRATBAG_EXPORT enum ratbag_error_code
 ratbag_button_set_button(struct ratbag_button *button, unsigned int btn)
 {
 	struct ratbag_button_action action = {0};
-	int rc;
 
 	if (!ratbag_device_has_capability(button->profile->device,
 					  RATBAG_DEVICE_CAP_BUTTON_KEY))
@@ -1009,12 +1084,11 @@ ratbag_button_set_button(struct ratbag_button *button, unsigned int btn)
 	action.type = RATBAG_BUTTON_ACTION_TYPE_BUTTON;
 	action.action.button = btn;
 
-	rc = button->profile->device->driver->write_button(button, &action);
+	button->action = action;
+	button->dirty = true;
+	button->profile->dirty = true;
 
-	if (rc == 0)
-		button->action = action;
-
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT enum ratbag_button_action_special
@@ -1031,7 +1105,6 @@ ratbag_button_set_special(struct ratbag_button *button,
 			  enum ratbag_button_action_special act)
 {
 	struct ratbag_button_action action = {0};
-	int rc;
 
 	/* FIXME: range checks */
 
@@ -1042,12 +1115,11 @@ ratbag_button_set_special(struct ratbag_button *button,
 	action.type = RATBAG_BUTTON_ACTION_TYPE_SPECIAL;
 	action.action.special = act;
 
-	rc = button->profile->device->driver->write_button(button, &action);
+	button->action = action;
+	button->dirty = true;
+	button->profile->dirty = true;
 
-	if (rc == 0)
-		button->action = action;
-
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT unsigned int
@@ -1070,7 +1142,6 @@ ratbag_button_set_key(struct ratbag_button *button,
 		      size_t sz)
 {
 	struct ratbag_button_action action = {0};
-	int rc;
 
 	/* FIXME: range checks */
 
@@ -1082,31 +1153,30 @@ ratbag_button_set_key(struct ratbag_button *button,
 
 	action.type = RATBAG_BUTTON_ACTION_TYPE_KEY;
 	action.action.key.key = key;
-	rc = button->profile->device->driver->write_button(button, &action);
 
-	if (rc == 0)
-		button->action = action;
+	button->action = action;
+	button->dirty = true;
+	button->profile->dirty = true;
 
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT enum ratbag_error_code
 ratbag_button_disable(struct ratbag_button *button)
 {
 	struct ratbag_button_action action;
-	int rc;
 
 	if (!ratbag_device_has_capability(button->profile->device,
 					  RATBAG_DEVICE_CAP_BUTTON_KEY))
 		return RATBAG_ERROR_CAPABILITY;
 
 	action.type = RATBAG_BUTTON_ACTION_TYPE_NONE;
-	rc = button->profile->device->driver->write_button(button, &action);
 
-	if (rc == 0)
-		button->action = action;
+	button->action = action;
+	button->dirty = true;
+	button->profile->dirty = true;
 
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT struct ratbag_button *
@@ -1245,17 +1315,15 @@ LIBRATBAG_EXPORT enum ratbag_error_code
 ratbag_button_set_macro(struct ratbag_button *button,
 			const struct ratbag_button_macro *macro)
 {
-	int rc;
-
 	if (!ratbag_device_has_capability(button->profile->device,
 					  RATBAG_DEVICE_CAP_BUTTON_MACROS))
 		return RATBAG_ERROR_CAPABILITY;
 
 	ratbag_button_copy_macro(button, macro);
+	button->dirty = true;
+	button->profile->dirty = true;
 
-	rc = button->profile->device->driver->write_button(button, &button->action);
-
-	return rc == 0 ? RATBAG_SUCCESS : RATBAG_ERROR_DEVICE;
+	return RATBAG_SUCCESS;
 }
 
 LIBRATBAG_EXPORT enum ratbag_error_code

src/libratbag.h

@@ -537,6 +537,18 @@ int
 ratbag_device_has_capability(const struct ratbag_device *device,
 			     enum ratbag_device_capability cap);
 
+/**
+ * @ingroup device
+ *
+ * Write any changes to the device. Depending on the device, this may take
+ * a couple of seconds.
+ *
+ * @param device A previously initialized ratbag device
+ * @return 0 on success or an error code otherwise
+ */
+enum ratbag_error_code
+ratbag_device_commit(struct ratbag_device *device);
+
 /**
  * @ingroup device
  *

src/libratbag.sym

@@ -1,6 +1,6 @@
 /* in alphabetical order! */
 
-LIBRATBAG_0.3.0 {
+LIBRATBAG_0.5.0 {
 global:
 	ratbag_create_context;
 	ratbag_button_disable;
@@ -28,6 +28,7 @@ global:
 	ratbag_button_set_special;
 	ratbag_button_set_user_data;
 	ratbag_button_unref;
+	ratbag_device_commit;
 	ratbag_device_get_name;
 	ratbag_device_get_num_buttons;
 	ratbag_device_get_num_profiles;

tools/ratbag-command.c

@@ -1701,6 +1701,10 @@ main(int argc, char **argv)
 			    ratbag,
 			    &options,
 			    argc, argv);
+
+	if (options.device)
+		rc = ratbag_device_commit(options.device);
+
 out:
 	ratbag_resolution_unref(options.resolution);
 	ratbag_button_unref(options.button);