From 0cd6727d452b5a58d6cd21a396bb6e86c9e55af4 Mon Sep 17 00:00:00 2001 From: Tim LaBerge Date: Mon, 17 Dec 2012 21:21:17 -0800 Subject: [PATCH] Fix review comments for pull request. 1) Don't expose gui mode to the causual user. Hide under the DEVELOPMENT_MODE #define. 2) Add accessors for the ModifierLinkableList's interesting fields. 3) Free the ModifierLinkableList from modifier_linkable_list(). This allows us to make the free function static. 4) Rename modifier_linkable_list() to update_modifier_linkable_list(). 5) In effects.h, reflect the renaming of update_modifier_linkable_list() and correct it's signature. 6) Whitespace fixups in gdigi.c. 7) In the handler for RECEIVE_GLOBAL_PARAMETERS, apply the parameter to the gui. 8) Fixup some merge damage ;-( 9) UnCamelCaseIze some variables. 10) When we create the gui, issue a single call to REQUEST_GLOBAL_PARAMETERS to retrieve the globals from the device, instead of polling each parameter individually. --- effects.c | 74 +++++++++++++++++++++++++++++++++++++------------------ effects.h | 5 ++-- gdigi.c | 30 ++++++++++++---------- gui.c | 37 +++++++++++----------------- 4 files changed, 85 insertions(+), 61 deletions(-) diff --git a/effects.c b/effects.c index c8654c0..3611561 100644 --- a/effects.c +++ b/effects.c @@ -723,7 +723,9 @@ static EffectValues values_rp_mix = { static EffectSettings global_settings[] = { {"USB/RP Mix", USB_AUDIO_PLAYBACK_MIX, GLOBAL_POSITION, &values_rp_mix}, {"USB Level", USB_AUDIO_LEVEL, GLOBAL_POSITION, &values_m12_to_24}, +#if defined(DEVELOPMENT_MODE) {"GUI Mode", GUI_MODE_ON_OFF, GLOBAL_POSITION, &values_on_off}, +#endif /* DEVELOPMENT_MODE */ {"Tuning Reference", TUNING_REFERENCE, GLOBAL_POSITION, &values_0_to_29}, {"Pedal Position", EXP_PEDAL_LEVEL, GLOBAL_POSITION, &values_0_to_255}, {"Stomp", STOMP_MODE, GLOBAL_POSITION, &values_on_off}, @@ -4308,12 +4310,52 @@ static void effect_settings_free(EffectSettings *settings) * Used for Pedal Assignment and the LFO's. */ ModifierGroup *ModifierLinkableList; + +EffectGroup *get_modifier_group(void) +{ + EffectGroup *group = NULL; + if (ModifierLinkableList) { + group = ModifierLinkableList->group; + } + return group; +} + +guint get_modifier_amt(void) +{ + guint amt = 0; + if (ModifierLinkableList) { + amt = ModifierLinkableList->group_amt; + } + return amt; +} + /** - * Retrieves modifier linkable group from device. + * \param modifier_group group to be freed * - * \return ModifierGroup which must be freed using modifier_group_free. + * Frees all memory used by ModifierGroup. **/ -ModifierGroup *modifier_linkable_list(GString *msg) +static void modifier_group_free(ModifierGroup *modifier_group) +{ + g_return_if_fail(modifier_group != NULL); + + int x; + for (x=0; xgroup_amt; x++) { + if (modifier_group->group[x].settings) + effect_settings_free(modifier_group->group[x].settings); + } + g_slice_free1(modifier_group->group_amt * sizeof(EffectGroup), + modifier_group->group); + g_slice_free(ModifierGroup, modifier_group); +} +/** + * Retrieves modifier linkable group from device and updates the + * global list of linkable parameters. The old list is freed. + * + * \param msg A buffer containing the message with the list of + * linkable parameters. + * + **/ +void update_modifier_linkable_list(GString *msg) { guint group_id; guint count; @@ -4362,30 +4404,14 @@ ModifierGroup *modifier_linkable_list(GString *msg) modifier_group->group = group; modifier_group->group_amt = count; - ModifierLinkableList = modifier_group; - - return modifier_group; -} - -/** - * \param modifier_group group to be freed - * - * Frees all memory used by ModifierGroup. - **/ -void modifier_group_free(ModifierGroup *modifier_group) -{ - g_return_if_fail(modifier_group != NULL); - - int x; - for (x=0; xgroup_amt; x++) { - if (modifier_group->group[x].settings) - effect_settings_free(modifier_group->group[x].settings); + if (ModifierLinkableList) { + modifier_group_free(ModifierLinkableList); + ModifierLinkableList = NULL; } - g_slice_free1(modifier_group->group_amt * sizeof(EffectGroup), - modifier_group->group); - g_slice_free(ModifierGroup, modifier_group); + ModifierLinkableList = modifier_group; } + /** * \param values EffectValues to examine * \param min return location for minimum value diff --git a/effects.h b/effects.h index 16fa0e1..59c7449 100644 --- a/effects.h +++ b/effects.h @@ -118,9 +118,10 @@ typedef struct { } Device; gchar *get_position(guint position); -ModifierGroup *modifier_linkable_list(); +void update_modifier_linkable_list(GString* msg); extern ModifierGroup *ModifierLinkableList; -void modifier_group_free(ModifierGroup *modifier_group); +EffectGroup *get_modifier_group(void); +guint get_modifier_amt(void); void get_values_info(EffectValues *values, gdouble *min, gdouble *max, gboolean *custom); gboolean get_device_info(unsigned char device_id, unsigned char family_id, diff --git a/gdigi.c b/gdigi.c index 800e9d2..fe8441d 100644 --- a/gdigi.c +++ b/gdigi.c @@ -52,7 +52,7 @@ gboolean set_debug_flags (const gchar *option_name, const gchar *value, { if (strchr(value, 'd')) { DebugFlags |= DEBUG_MSG2DEV; - } + } if (strchr(value, 'h')) { DebugFlags |= DEBUG_MSG2HOST; } @@ -91,7 +91,7 @@ debug_msg (debug_flags_t flags, char *fmt, ...) va_start(ap, fmt); vsnprintf(buf, 1024, fmt, ap); va_end(ap); - + fprintf(stderr, "%s\n", buf); } } @@ -460,7 +460,7 @@ void push_message(GString *msg) "%d from bank %d", str[10], str[9]); } else { - debug_msg(DEBUG_MSG2HOST, + debug_msg(DEBUG_MSG2HOST, "RECEIVE_DEVICE_NOTIFICATION: %d %d moved to " "%d %d", str[9], str[10], @@ -479,16 +479,11 @@ void push_message(GString *msg) printf("\n"); } - debug_msg(DEBUG_MSG2HOST, + debug_msg(DEBUG_MSG2HOST, "NOTIFY_MODIFIER_GROUP_CHANGED: Modifier group " "id %d changed", (str[9] << 8) | (str[10])); - if (ModifierLinkableList) { - modifier_group_free(ModifierLinkableList); - ModifierLinkableList = NULL; - } - send_message(REQUEST_MODIFIER_LINKABLE_LIST, "\x00\x01", 2); break; } @@ -518,9 +513,14 @@ void push_message(GString *msg) "Position: %2.1d Value: %6.1d: %s", param->id, param->position, param->value, "XXX"); + + GDK_THREADS_ENTER(); + apply_setting_param_to_gui(param); + GDK_THREADS_LEAVE(); + setting_param_free(param); } while ( (x < msg->len) && n < tot); - + g_string_free(msg, TRUE); return; @@ -536,8 +536,8 @@ void push_message(GString *msg) printf("\n"); } - modifier_linkable_list(msg); - + update_modifier_linkable_list(msg); + g_string_free(msg, TRUE); GDK_THREADS_ENTER(); @@ -1365,6 +1365,10 @@ static gboolean request_who_am_i(unsigned char *device_id, unsigned char *family *family_id = data->str[9]; *product_id = data->str[10]; g_string_free(data, TRUE); + debug_msg(DEBUG_STARTUP, "Found device id %d family %d product id %d.", + *device_id, + *family_id, + *product_id); return TRUE; } return FALSE; @@ -1487,7 +1491,7 @@ int main(int argc, char *argv[]) { GList *device = NULL; int num_devices = 0; int chosen_device = 0; - if (get_digitech_devices(&devices) <= 0) { + if ((num_devices = get_digitech_devices(&devices)) <= 0) { g_warning("Couldn't find DigiTech devices!"); exit(EXIT_FAILURE); } diff --git a/gui.c b/gui.c index 85c622b..7bae8f7 100644 --- a/gui.c +++ b/gui.c @@ -683,10 +683,12 @@ GtkWidget *create_widget_container(EffectGroup *group, gint amt, gint id, gint p /** * Populate a combo box with text entries from the modifier group. */ -void update_modifier_combo_box(GObject *combo_box, EffectGroup *group, gint amt, gint id, gint position) +void update_modifier_combo_box(GObject *combo_box, gint id, gint position) { gint x; EffectSettingsGroup *settings = NULL; + EffectGroup *group = get_modifier_group(); + guint amt = get_modifier_amt(); for (x = 0; xwidget); + g_assert(combo_box == el->widget); name = g_strdup_printf("SettingsGroup%d", el->x); - settings = g_object_steal_data(G_OBJECT(ComboBox), name); + settings = g_object_steal_data(G_OBJECT(combo_box), name); g_free(name); g_slice_free(EffectSettingsGroup, settings); @@ -737,7 +739,7 @@ static void clean_modifier_combo_box (GObject *ComboBox, GList *list) } link = next; } - gtk_combo_box_text_remove_all(GTK_COMBO_BOX_TEXT(ComboBox)); + gtk_combo_box_text_remove_all(GTK_COMBO_BOX_TEXT(combo_box)); } /** @@ -754,7 +756,7 @@ create_modifier_group (guint pos, guint id) gpointer key; WidgetTreeElem *el; GList *list; - GObject *AssignComboBox; + GObject *modifier_combo_box; debug_msg(DEBUG_GROUP, "Building modifier group for position %d id %d \"%s\"", pos, id, get_xml_settings(id, pos)->label); @@ -780,18 +782,15 @@ create_modifier_group (guint pos, guint id) return; } - AssignComboBox = el->widget; - g_assert(AssignComboBox != NULL); + modifier_combo_box = el->widget; + g_assert(modifier_combo_box != NULL); - vbox = g_object_get_data(AssignComboBox, "vbox"); + vbox = g_object_get_data(modifier_combo_box, "vbox"); g_assert(vbox != NULL); - clean_modifier_combo_box(AssignComboBox, list); + clean_modifier_combo_box(modifier_combo_box, list); - update_modifier_combo_box(AssignComboBox, - ModifierLinkableList->group, - ModifierLinkableList->group_amt, - id, pos); + update_modifier_combo_box(modifier_combo_box, id, pos); get_option(id, pos); } @@ -1508,15 +1507,9 @@ void gui_create(Device *device) g_signal_connect(G_OBJECT(window), "delete_event", G_CALLBACK(gtk_main_quit), NULL); - /* Not part of the preset, but update from the device. */ - get_option(TUNING_REFERENCE, GLOBAL_POSITION); - get_option(USB_AUDIO_PLAYBACK_MIX, GLOBAL_POSITION); - get_option(GUI_MODE_ON_OFF, GLOBAL_POSITION); - get_option(USB_AUDIO_LEVEL, GLOBAL_POSITION); - get_option(EXP_PEDAL_LEVEL, GLOBAL_POSITION); - get_option(STOMP_MODE, GLOBAL_POSITION); - + /* Get the initial values for the linkable parameters and the globals. */ send_message(REQUEST_MODIFIER_LINKABLE_LIST, "\x00\x01", 2); + send_message(REQUEST_GLOBAL_PARAMETERS, "\x00\x01", 2); } /**