We have to keep valueChangedFromEngine. It's important to be able to react
to internal sets of controls. For example this breaks BpmControl reacting
to engine rate changes. Try hitting the sync button. The BPM text won't
change.
On May 16, 2013 7:49 PM, "Daniel Schürmann" <email address hidden> wrote:

> Daniel Schürmann has proposed merging lp:~daschuer/mixxx/atomic-co into
> lp:~mixxxdevelopers/mixxx/atomic-co.
>
> Requested reviews:
>   Mixxx Development Team (mixxxdevelopers)
>
> For more details, see:
> https://code.launchpad.net/~daschuer/mixxx/atomic-co/+merge/164281
>
> See commit log
> --
> https://code.launchpad.net/~daschuer/mixxx/atomic-co/+merge/164281
> Your team Mixxx Development Team is requested to review the proposed merge
> of lp:~daschuer/mixxx/atomic-co into lp:~mixxxdevelopers/mixxx/atomic-co.
>
> === modified file 'mixxx/src/control/control.cpp'
> --- mixxx/src/control/control.cpp       2013-05-16 01:40:06 +0000
> +++ mixxx/src/control/control.cpp       2013-05-16 23:48:28 +0000
> @@ -77,9 +77,12 @@
>      return m_value.getValue();
>  }
>
> -void ControlDoublePrivate::reset(QObject* pSender) {
> +void ControlDoublePrivate::reset() {
>      double defaultValue = m_defaultValue.getValue();
> -    set(defaultValue, pSender);
> +    // NOTE: pSender = NULL is important. The originator of this action
> does
> +    // not know the resulting value so it makes sense that we should emit
> a
> +    // general valueChanged() signal even though we know the originator.
> +    set(defaultValue, NULL);
>  }
>
>  void ControlDoublePrivate::set(const double& value, QObject* pSender) {
> @@ -106,9 +109,9 @@
>      return m_pBehavior.fetchAndStoreRelaxed(pBehavior);
>  }
>
> -void ControlDoublePrivate::setWidgetParameter(double dParam, QObject*
> pSetter) {
> +void ControlDoublePrivate::setWidgetParameter(double dParam, QObject*
> pSender) {
>      ControlNumericBehavior* pBehavior = m_pBehavior;
> -    set(pBehavior ? pBehavior->widgetParameterToValue(dParam) : dParam,
> pSetter);
> +    set(pBehavior ? pBehavior->widgetParameterToValue(dParam) : dParam,
> pSender);
>  }
>
>  double ControlDoublePrivate::getWidgetParameter() const {
> @@ -116,12 +119,12 @@
>      return pBehavior ? pBehavior->valueToWidgetParameter(get()) : get();
>  }
>
> -void ControlDoublePrivate::setMidiParameter(MidiOpCode opcode, double
> dParam) {
> +void ControlDoublePrivate::setMidiParameter(MidiOpCode opcode, double
> dParam, QObject* pSender) {
>      ControlNumericBehavior* pBehavior = m_pBehavior;
>      if (pBehavior) {
> -        pBehavior->setValueFromMidiParameter(opcode, dParam, this);
> +        pBehavior->setValueFromMidiParameter(opcode, dParam, this,
> pSender);
>      } else {
> -        set(dParam, NULL);
> +        set(dParam, pSender);
>      }
>  }
>
>
> === modified file 'mixxx/src/control/control.h'
> --- mixxx/src/control/control.h 2013-05-16 01:40:06 +0000
> +++ mixxx/src/control/control.h 2013-05-16 23:48:28 +0000
> @@ -32,11 +32,11 @@
>      }
>
>      // Sets the control value.
> -    void set(const double& value, QObject* pSetter);
> +    void set(const double& value, QObject* pSender);
>      // Gets the control value.
>      double get() const;
>      // Resets the control value to its default.
> -    void reset(QObject* pSetter);
> +    void reset();
>
>      // Set the behavior to be used when setting values and translating
> between
>      // parameter and value space. Returns the previously set behavior (if
> any).
> @@ -44,10 +44,10 @@
>      // memory will leak.
>      ControlNumericBehavior* setBehavior(ControlNumericBehavior*
> pBehavior);
>
> -    void setWidgetParameter(double dParam, QObject* pSetter);
> +    void setWidgetParameter(double dParam, QObject* pSender);
>      double getWidgetParameter() const;
>
> -    void setMidiParameter(MidiOpCode opcode, double dParam);
> +    void setMidiParameter(MidiOpCode opcode, double dParam, QObject*
> pSender);
>      double getMidiParameter() const;
>
>      inline bool ignoreNops() const {
> @@ -63,9 +63,9 @@
>      }
>
>    signals:
> -    // Emitted when the ControlDoublePrivate value changes. pSetter is a
> +    // Emitted when the ControlDoublePrivate value changes. pSender is a
>      // pointer to the setter of the value (potentially NULL).
> -    void valueChanged(double value, QObject* pSetter);
> +    void valueChanged(double value, QObject* pSender);
>
>    private:
>      ConfigKey m_key;
>
> === modified file 'mixxx/src/control/controlbehavior.cpp'
> --- mixxx/src/control/controlbehavior.cpp       2013-05-16 01:40:06 +0000
> +++ mixxx/src/control/controlbehavior.cpp       2013-05-16 23:48:28 +0000
> @@ -5,59 +5,65 @@
>  const int ControlPushButtonBehavior::kPowerWindowTimeMillis = 300;
>
>  void ControlNumericBehavior::setValueFromMidiParameter(MidiOpCode o,
> double dParam,
> -
> ControlDoublePrivate* pControl) {
> +
> ControlDoublePrivate* pControl,
> +                                                       QObject* pSender) {
>      Q_UNUSED(o);
> -    pControl->set(dParam, NULL);
> +    pControl->set(dParam, pSender);
>  }
>
>  void ControlPotmeterBehavior::setValueFromMidiParameter(MidiOpCode o,
> double dParam,
> -
>  ControlDoublePrivate* pControl) {
> +
>  ControlDoublePrivate* pControl,
> +                                                        QObject* pSender)
> {
>      Q_UNUSED(o);
> -    pControl->set(widgetParameterToValue(dParam), NULL);
> +    pControl->set(widgetParameterToValue(dParam), pSender);
>  }
>
>  void ControlPushButtonBehavior::setValueFromMidiParameter(
> -    MidiOpCode o, double dParam, ControlDoublePrivate* pControl) {
> +        MidiOpCode o, double dParam, ControlDoublePrivate* pControl,
> QObject* pSender) {
>      // This block makes push-buttons act as power window buttons.
>      if (m_buttonMode == POWERWINDOW && m_iNumStates == 2) {
>          if (o == MIDI_NOTE_ON) {
>              if (dParam > 0.) {
>                  double value = pControl->get();
> -                pControl->set(!value, NULL);
> +                pControl->set(!value, pSender);
>                  m_pushTimer.setSingleShot(true);
>                  m_pushTimer.start(kPowerWindowTimeMillis);
>              }
>          } else if (o == MIDI_NOTE_OFF) {
>              if (!m_pushTimer.isActive()) {
> -                pControl->set(0.0, NULL);
> +                pControl->set(0.0, pSender);
>              }
>          }
>      } else if (m_buttonMode == TOGGLE) {
>          // This block makes push-buttons act as toggle buttons.
> -        if (m_iNumStates > 2) { //multistate button
> -            if (dParam > 0.) { //looking for NOTE_ON doesn't seem to
> work...
> +        if (m_iNumStates > 2) { // multistate button
> +            if (dParam > 0.) { // looking for NOTE_ON doesn't seem to
> work...
> +                // This is a possible race condition if an other writer
> wants
> +                // to change the value at the same time. We allow the
> race her,
> +                // because this is possible what the user expect if he
> controls
> +                // the same control from different devices.
>                  double value = pControl->get();
>                  value++;
>                  if (value >= m_iNumStates) {
> -                    pControl->set(0, NULL);
> +                    pControl->set(0, pSender);
>                  } else {
> -                    pControl->set(value, NULL);
> +                    pControl->set(value, pSender);
>                  }
>              }
>          } else {
>              if (o == MIDI_NOTE_ON) {
>                  if (dParam > 0.) {
>                      double value = pControl->get();
> -                    pControl->set(!value, NULL);
> +                    pControl->set(!value, pSender);
>                  }
>              }
>          }
>      } else { //Not a toggle button (trigger only when button pushed)
>          if (o == MIDI_NOTE_ON) {
>              double value = pControl->get();
> -            pControl->set(!value, NULL);
> +            pControl->set(!value, pSender);
>          } else if (o == MIDI_NOTE_OFF) {
> -            pControl->set(0.0, NULL);
> +            pControl->set(0.0, pSender);
>          }
>      }
>  }
>
> === modified file 'mixxx/src/control/controlbehavior.h'
> --- mixxx/src/control/controlbehavior.h 2013-05-16 01:40:06 +0000
> +++ mixxx/src/control/controlbehavior.h 2013-05-16 23:48:28 +0000
> @@ -36,7 +36,8 @@
>      }
>
>      virtual void setValueFromMidiParameter(MidiOpCode o, double dParam,
> -                                           ControlDoublePrivate*
> pControl);
> +                                           ControlDoublePrivate* pControl,
> +                                           QObject* pSender);
>  };
>
>  class ControlPotmeterBehavior : public ControlNumericBehavior {
> @@ -77,7 +78,8 @@
>      }
>
>      virtual void setValueFromMidiParameter(MidiOpCode o, double dParam,
> -                                           ControlDoublePrivate*
> pControl);
> +                                           ControlDoublePrivate* pControl,
> +                                           QObject* pSender);
>
>    protected:
>      double m_dMinValue;
> @@ -175,7 +177,8 @@
>      }
>
>      virtual void setValueFromMidiParameter(MidiOpCode o, double dParam,
> -                                           ControlDoublePrivate*
> pControl);
> +                                           ControlDoublePrivate* pControl,
> +                                           QObject* pSender);
>
>    private:
>      ButtonMode m_buttonMode;
>
> === modified file 'mixxx/src/controllers/midi/midicontroller.cpp'
> --- mixxx/src/controllers/midi/midicontroller.cpp       2013-05-16
> 00:14:12 +0000
> +++ mixxx/src/controllers/midi/midicontroller.cpp       2013-05-16
> 23:48:28 +0000
> @@ -356,7 +356,7 @@
>                  return;
>              }
>          }
> -        p->setValueFromThread(newValue, NULL);
> +        p->set(newValue);
>      } else {
>          if (options.soft_takeover) {
>              if (m_st.ignore(p, newValue, true)) {
>
> === modified file 'mixxx/src/controlobject.cpp'
> --- mixxx/src/controlobject.cpp 2013-05-16 01:40:06 +0000
> +++ mixxx/src/controlobject.cpp 2013-05-16 23:48:28 +0000
> @@ -67,12 +67,11 @@
>      m_sqCOHashMutex.unlock();
>  }
>
> -void ControlObject::privateValueChanged(double dValue, QObject* pSetter) {
> +// slot
> +void ControlObject::privateValueChanged(double dValue, QObject* pSender) {
>      // Only emit valueChanged() if we did not originate this change.
> -    if (pSetter != this) {
> +    if (pSender != this) {
>          emit(valueChanged(dValue));
> -    } else {
> -        emit(valueChangedFromEngine(dValue));
>      }
>  }
>
> @@ -129,7 +128,7 @@
>
>  void ControlObject::setValueFromMidi(MidiOpCode o, double v) {
>      if (m_pControl) {
> -        m_pControl->setMidiParameter(o, v);
> +        m_pControl->setMidiParameter(o, v, this);
>      }
>  }
>
> @@ -137,19 +136,13 @@
>      return m_pControl ? m_pControl->getMidiParameter() : 0.0;
>  }
>
> -void ControlObject::setValueFromThread(double dValue, QObject* pSender) {
> -    if (m_pControl) {
> -        m_pControl->set(dValue, pSender);
> -    }
> -}
> -
>  double ControlObject::get() const {
>      return m_pControl ? m_pControl->get() : 0.0;
>  }
>
>  void ControlObject::reset() {
>      if (m_pControl) {
> -        m_pControl->reset(this);
> +        m_pControl->reset();
>      }
>  }
>
>
> === modified file 'mixxx/src/controlobject.h'
> --- mixxx/src/controlobject.h   2013-05-16 01:40:06 +0000
> +++ mixxx/src/controlobject.h   2013-05-16 23:48:28 +0000
> @@ -66,15 +66,12 @@
>
>    signals:
>      void valueChanged(double);
> -    void valueChangedFromEngine(double);
>
>    public:
>      // DEPRECATED: Called to set the control value from the controller
>      // subsystem.
>      virtual void setValueFromMidi(MidiOpCode o, double v);
>      virtual double getValueToMidi() const;
> -    // DEPRECATED: Called to set the control value from another thread.
> -    virtual void setValueFromThread(double dValue, QObject* pSetter);
>
>    protected:
>      // Key of the object
>
> === modified file 'mixxx/src/controlobjectthread.cpp'
> --- mixxx/src/controlobjectthread.cpp   2013-05-16 01:40:06 +0000
> +++ mixxx/src/controlobjectthread.cpp   2013-05-16 23:48:28 +0000
> @@ -64,7 +64,7 @@
>          // general valueChanged() signal even though the change
> originated from
>          // us. For this reason, we provide NULL here so that the change is
>          // broadcast as valueChanged() and not valueChangedByThis().
> -        m_pControl->reset(NULL);
> +        m_pControl->reset();
>      }
>  }
>
>
> === modified file 'mixxx/src/engine/bpmcontrol.cpp'
> --- mixxx/src/engine/bpmcontrol.cpp     2013-05-15 04:41:08 +0000
> +++ mixxx/src/engine/bpmcontrol.cpp     2013-05-16 23:48:28 +0000
> @@ -26,25 +26,16 @@
>      connect(m_pRateSlider, SIGNAL(valueChanged(double)),
>              this, SLOT(slotAdjustBpm()),
>              Qt::DirectConnection);
> -    connect(m_pRateSlider, SIGNAL(valueChangedFromEngine(double)),
> -            this, SLOT(slotAdjustBpm()),
> -            Qt::DirectConnection);
>
>      m_pRateRange = ControlObject::getControl(ConfigKey(_group,
> "rateRange"));
>      connect(m_pRateRange, SIGNAL(valueChanged(double)),
>              this, SLOT(slotAdjustBpm()),
>              Qt::DirectConnection);
> -    connect(m_pRateRange, SIGNAL(valueChangedFromEngine(double)),
> -            this, SLOT(slotAdjustBpm()),
> -            Qt::DirectConnection);
>
>      m_pRateDir = ControlObject::getControl(ConfigKey(_group, "rate_dir"));
>      connect(m_pRateDir, SIGNAL(valueChanged(double)),
>              this, SLOT(slotAdjustBpm()),
>              Qt::DirectConnection);
> -    connect(m_pRateDir, SIGNAL(valueChangedFromEngine(double)),
> -            this, SLOT(slotAdjustBpm()),
> -            Qt::DirectConnection);
>
>      m_pLoopEnabled = ControlObject::getControl(
>          ConfigKey(_group, "loop_enabled"));
>
> === modified file 'mixxx/src/engine/enginebuffer.cpp'
> --- mixxx/src/engine/enginebuffer.cpp   2013-05-15 04:41:08 +0000
> +++ mixxx/src/engine/enginebuffer.cpp   2013-05-16 23:48:28 +0000
> @@ -145,9 +145,6 @@
>      connect(m_pSlipButton, SIGNAL(valueChanged(double)),
>              this, SLOT(slotControlSlip(double)),
>              Qt::DirectConnection);
> -    connect(m_pSlipButton, SIGNAL(valueChangedFromEngine(double)),
> -            this, SLOT(slotControlSlip(double)),
> -            Qt::DirectConnection);
>      m_pSlipPosition = new ControlObject(ConfigKey(m_group,
> "slip_playposition"));
>
>      // Actual rate (used in visuals, not for control)
>
> === modified file 'mixxx/src/engine/ratecontrol.cpp'
> --- mixxx/src/engine/ratecontrol.cpp    2013-05-15 23:01:31 +0000
> +++ mixxx/src/engine/ratecontrol.cpp    2013-05-16 23:48:28 +0000
> @@ -153,9 +153,6 @@
>      connect(pVCEnabled, SIGNAL(valueChanged(double)),
>              this, SLOT(slotControlVinyl(double)),
>              Qt::DirectConnection);
> -    connect(pVCEnabled, SIGNAL(valueChangedFromEngine(double)),
> -            this, SLOT(slotControlVinyl(double)),
> -            Qt::DirectConnection);
>
>      ControlObject* pVCScratching =
> ControlObject::getControl(ConfigKey(_group, "vinylcontrol_scratching"));
>      // Throw a hissy fit if somebody moved us such that the
> vinylcontrol_enabled
> @@ -164,9 +161,6 @@
>      connect(pVCScratching, SIGNAL(valueChanged(double)),
>              this, SLOT(slotControlVinylScratching(double)),
>              Qt::DirectConnection);
> -    connect(pVCScratching, SIGNAL(valueChangedFromEngine(double)),
> -            this, SLOT(slotControlVinylScratching(double)),
> -            Qt::DirectConnection);
>  #endif
>  }
>
>
> === modified file 'mixxx/src/vinylcontrol/vinylcontrolmanager.cpp'
> --- mixxx/src/vinylcontrol/vinylcontrolmanager.cpp      2013-05-14
> 04:40:32 +0000
> +++ mixxx/src/vinylcontrol/vinylcontrolmanager.cpp      2013-05-16
> 23:48:28 +0000
> @@ -59,22 +59,21 @@
>  {
>      // Load saved preferences now that the objects exist
>
>  ControlObject::getControl(ConfigKey("[Channel1]","vinylcontrol_enabled"))
> -            ->setValueFromThread(0, NULL);
> +            ->set(0);
>
>  ControlObject::getControl(ConfigKey("[Channel2]","vinylcontrol_enabled"))
> -            ->setValueFromThread(0, NULL);
> -
> +            ->set(0);
>      ControlObject::getControl(ConfigKey("[Channel1]","vinylcontrol_mode"))
> -            ->setValueFromThread(m_pConfig->getValueString(
> -                ConfigKey("[VinylControl]","mode")).toDouble(), NULL);
> +            ->set(m_pConfig->getValueString(
> +                ConfigKey("[VinylControl]","mode")).toDouble());
>      ControlObject::getControl(ConfigKey("[Channel2]","vinylcontrol_mode"))
> -            ->setValueFromThread(m_pConfig->getValueString(
> -                ConfigKey("[VinylControl]","mode")).toDouble(), NULL);
> +            ->set(m_pConfig->getValueString(
> +                ConfigKey("[VinylControl]","mode")).toDouble());
>
>  ControlObject::getControl(ConfigKey("[Channel1]","vinylcontrol_cueing"))
> -            ->setValueFromThread(m_pConfig->getValueString(
> -                ConfigKey("[VinylControl]","cueing_ch1")).toDouble(),
> NULL);
> +            ->set(m_pConfig->getValueString(
> +                ConfigKey("[VinylControl]","cueing_ch1")).toDouble());
>
>  ControlObject::getControl(ConfigKey("[Channel2]","vinylcontrol_cueing"))
> -            ->setValueFromThread(m_pConfig->getValueString(
> -                ConfigKey("[VinylControl]","cueing_ch2")).toDouble(),
> NULL);
> +            ->set(m_pConfig->getValueString(
> +                ConfigKey("[VinylControl]","cueing_ch2")).toDouble());
>  }
>
>  void VinylControlManager::receiveBuffer(AudioInput input,
>
>
>