Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CO* constructor to COT. Cleanups. #16

Merged
merged 22 commits into from
Jun 20, 2013
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8afddd3
removed valueChangedFromEngine()
daschuer May 16, 2013
33dfbf9
unified word choice for pSender, pSetter is the a function pointer to…
daschuer May 16, 2013
bc44569
added pSender to setMidiParameter
daschuer May 16, 2013
78af330
removed setValueFromThread
daschuer May 16, 2013
e6fdaaa
removed pSender from ControlDoublePrivate::reset()
daschuer May 16, 2013
f16f136
merged from lp:mixxx
daschuer May 21, 2013
f0cfd08
merged from lp:mixxx
daschuer May 23, 2013
aac8856
merged from lp:mixxx
daschuer Jun 9, 2013
98e895e
merged from lp:~mixxxdevelopers/mixxx/atomic-co
daschuer Jun 9, 2013
8f2c854
Merge branch 'master' into atomic-co
daschuer Jun 16, 2013
059fa5b
some refactoring
daschuer Jun 17, 2013
ec04156
get rid of ControlObect dependency of ControlObjectThread
daschuer Jun 18, 2013
3df0301
Merge remote-tracking branch 'upstream/master' into atomic-co
daschuer Jun 18, 2013
41afff9
get rif of ControlObject dependency of ControlObjectThreadMain
daschuer Jun 18, 2013
c802cef
get rid of ControlObject dependency of ControlObjectThreadWidget
daschuer Jun 18, 2013
a099961
transfer ConfigKey via reference
daschuer Jun 18, 2013
56b7709
Merge remote-tracking branch 'upstream/master' into atomic-co
daschuer Jun 19, 2013
4523cfd
bc44569 partial reverted, for a merge request, focused on COT refacto…
daschuer Jun 19, 2013
076f48e
changes from RJs comments
daschuer Jun 20, 2013
bb707b1
repace setValueFromThread by set via a tamporary cot
daschuer Jun 20, 2013
6fab199
another spelling fix
daschuer Jun 20, 2013
e9510b9
added valueChangedFromEngine connections back in
daschuer Jun 20, 2013
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions src/basetrackplayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ BaseTrackPlayer::BaseTrackPlayer(QObject* pParent,
// Set the routing option defaults for the master and headphone mixes.
{
ControlObjectThreadMain* pMaster = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(getGroup(), "master")));
getGroup(), "master");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indentation (8-space indent) doesn't match the rest of Mixxx -- please use 4-space indents.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please! .. let us (slowly) change this to 8-space indent for line wrap!

  • The code is much more easy to read. Since we have the braces at the and of the line, you can instantly distinguish between a line wrap and a conditional code block. See "src/control/control.cpp @@ -49,7 +49,7 @@" below.

Here what other think:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, it's the right style (if Google style had 4-spaces as default indentation) -- sorry about that.

pMaster->slotSet(defaultMaster);
delete pMaster;

ControlObjectThreadMain* pHeadphones = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(getGroup(), "pfl")));
getGroup(), "pfl");
pHeadphones->slotSet(defaultHeadphones);
delete pHeadphones;
}
Expand All @@ -74,15 +74,15 @@ BaseTrackPlayer::BaseTrackPlayer(QObject* pParent,

//Get cue point control object
m_pCuePoint = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(getGroup(),"cue_point")));
getGroup(),"cue_point");
// Get loop point control objects
m_pLoopInPoint = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(getGroup(),"loop_start_position")));
getGroup(),"loop_start_position");
m_pLoopOutPoint = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(getGroup(),"loop_end_position")));
getGroup(),"loop_end_position");
//Playback position within the currently loaded track (in this player).
m_pPlayPosition = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(getGroup(), "playposition")));
getGroup(), "playposition");

// Duration of the current song, we create this one because nothing else does.
m_pDuration = new ControlObject(ConfigKey(getGroup(), "duration"));
Expand All @@ -99,14 +99,9 @@ BaseTrackPlayer::BaseTrackPlayer(QObject* pParent,
m_pEndOfTrack->set(0.);

//BPM of the current song
m_pBPM = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(group, "file_bpm")));

m_pReplayGain = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(group, "replaygain")));

m_pPlay = new ControlObjectThreadMain(
ControlObject::getControl(ConfigKey(group, "play")));
m_pBPM = new ControlObjectThreadMain(group, "file_bpm");
m_pReplayGain = new ControlObjectThreadMain(group, "replaygain");
m_pPlay = new ControlObjectThreadMain(group, "play");
}

BaseTrackPlayer::~BaseTrackPlayer()
Expand Down
15 changes: 9 additions & 6 deletions src/configobject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,17 @@

#include "mixxx.h"

ConfigKey::ConfigKey()
{
ConfigKey::ConfigKey() {
}

ConfigKey::ConfigKey(QString g, QString i)
{
group = g;
item = i;
ConfigKey::ConfigKey(const QString& g, const QString& i)
: group(g),
item(i) {
}

ConfigKey::ConfigKey(const char* g, const char* i)
: group(g),
item(i) {
}

// static
Expand Down
21 changes: 9 additions & 12 deletions src/configobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ typedef QMap<char,char> MidiValueMap;
Class for the key for a specific configuration element. A key consists of a
group and an item.
*/
class ConfigKey
{
public:
class ConfigKey {
public:
ConfigKey();
ConfigKey(QString g, QString i);
ConfigKey(const QString& g, const QString& i);
ConfigKey(const char* g, const char* i);
static ConfigKey parseCommaSeparated(QString key);
QString group, item;
};
Expand All @@ -65,9 +65,8 @@ inline uint qHash(const ConfigKey &key) {
The value corresponding to a key. The basic value is a string, but can be
subclassed to more specific needs.
*/
class ConfigValue
{
public:
class ConfigValue {
public:
ConfigValue();
ConfigValue(QString _value);
ConfigValue(int _value);
Expand All @@ -79,9 +78,8 @@ class ConfigValue
};


class ConfigValueKbd : public ConfigValue
{
public:
class ConfigValueKbd : public ConfigValue {
public:
ConfigValueKbd();
ConfigValueKbd(QString _value);
ConfigValueKbd(QKeySequence key);
Expand All @@ -92,8 +90,7 @@ class ConfigValueKbd : public ConfigValue
QKeySequence m_qKey;
};

template <class ValueType> class ConfigOption
{
template <class ValueType> class ConfigOption {
public:
ConfigOption() { val = NULL; key = NULL;};
ConfigOption(ConfigKey *_key, ValueType *_val) { key = _key ; val = _val; };
Expand Down
13 changes: 8 additions & 5 deletions src/control/control.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ ControlDoublePrivate::~ControlDoublePrivate() {

// static
ControlDoublePrivate* ControlDoublePrivate::getControl(
const ConfigKey& key, bool bCreate, bool bIgnoreNops, bool bTrack) {
const ConfigKey& key, bool bCreate, bool bIgnoreNops, bool bTrack) {
QMutexLocker locker(&m_sqCOHashMutex);
QHash<ConfigKey, ControlDoublePrivate*>::const_iterator it = m_sqCOHash.find(key);
if (it != m_sqCOHash.end()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here you can see th advantage of double indentation line wrap.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, this has annoyed me for a long time too but for consistency with the rest of the code I was sticking to 4-space indent.

Expand Down Expand Up @@ -77,9 +77,12 @@ double ControlDoublePrivate::get() const {
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) {
Expand All @@ -106,9 +109,9 @@ ControlNumericBehavior* ControlDoublePrivate::setBehavior(ControlNumericBehavior
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 {
Expand Down
10 changes: 5 additions & 5 deletions src/control/control.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ class ControlDoublePrivate : public QObject {
}

// 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).
// Caller must handle appropriate destruction of the previous behavior or
// 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);
Expand All @@ -63,9 +63,9 @@ class ControlDoublePrivate : public QObject {
}

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;
Expand Down
10 changes: 7 additions & 3 deletions src/control/controlbehavior.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ ControlPushButtonBehavior::ControlPushButtonBehavior(ButtonMode buttonMode,
}

void ControlPushButtonBehavior::setValueFromMidiParameter(
MidiOpCode o, double dParam, ControlDoublePrivate* pControl) {
MidiOpCode o, double dParam, ControlDoublePrivate* pControl) {
// This block makes push-buttons act as power window buttons.
if (m_buttonMode == POWERWINDOW && m_iNumStates == 2) {
if (o == MIDI_NOTE_ON) {
Expand All @@ -178,8 +178,12 @@ void ControlPushButtonBehavior::setValueFromMidiParameter(
}
} 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 possibly race condition if another writer wants
// to change the value at the same time. We allow the race here,
// because this is possible what the user expects if he changes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/possible/possibly/

// the same control from different devices.
double value = pControl->get();
value++;
if (value >= m_iNumStates) {
Expand Down
2 changes: 2 additions & 0 deletions src/control/controlbehavior.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ class ControlDoublePrivate;

class ControlNumericBehavior {
public:
virtual ~ControlNumericBehavior() { };

// Returns true if the set should occur. Mutates dValue if the value should
// be changed.
virtual bool setFilter(double* dValue);
Expand Down
17 changes: 8 additions & 9 deletions src/controllers/controllerengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,18 +597,17 @@ void ControllerEngine::errorDialogButton(QString key, QMessageBox::StandardButto

ControlObjectThread* ControllerEngine::getControlObjectThread(QString group, QString name) {
ConfigKey key = ConfigKey(group, name);

ControlObjectThread *cot = NULL;
if(!m_controlCache.contains(key)) {
ControlObject *co = ControlObject::getControl(key);
if(co != NULL) {
cot = new ControlObjectThread(co);
ControlObjectThread* cot = m_controlCache.value(key, NULL);
if (cot == NULL) {
// create COT
cot = new ControlObjectThread(key);
if (cot->valid()) {
m_controlCache.insert(key, cot);
} else {
delete cot;
cot = NULL;
}
} else {
cot = m_controlCache.value(key);
}

return cot;
}

Expand Down
18 changes: 10 additions & 8 deletions src/controllers/midi/midicontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ void MidiController::receive(unsigned char status, unsigned char control,
}

// Only pass values on to valid ControlObjects.
ControlObject* p = mc.getControlObject();
if (p == NULL) {
ControlObject* pCO = mc.getControlObject();
if (pCO == NULL) {
return;
}

Expand All @@ -334,7 +334,7 @@ void MidiController::receive(unsigned char status, unsigned char control,

// computeValue not (yet) done on pitch messages because it all assumes 7-bit numbers
} else {
double currMixxxControlValue = p->getValueToMidi();
double currMixxxControlValue = pCO->getValueToMidi();
newValue = computeValue(options, currMixxxControlValue, value);
}

Expand All @@ -346,24 +346,26 @@ void MidiController::receive(unsigned char status, unsigned char control,

if (options.soft_takeover) {
// This is the only place to enable it if it isn't already.
m_st.enable(p);
m_st.enable(pCO);
}

if (opCode == MIDI_PITCH_BEND) {
// Absolute value is calculated above on Pitch messages (-1..1)
if (options.soft_takeover) {
if (m_st.ignore(p, newValue, false)) {
if (m_st.ignore(pCO, newValue, false)) {
return;
}
}
p->setValueFromThread(newValue, NULL);
// Use temporary cot for bypass own signal filter
ControlObjectThread cot(pCO->getKey());
cot.set(newValue);
} else {
if (options.soft_takeover) {
if (m_st.ignore(p, newValue, true)) {
if (m_st.ignore(pCO, newValue, true)) {
return;
}
}
p->setValueFromMidi(static_cast<MidiOpCode>(opCode), newValue);
pCO->setValueFromMidi(static_cast<MidiOpCode>(opCode), newValue);
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/controllers/midi/midioutputhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,38 @@
#include "controllers/midi/midicontroller.h"
#include "controlobject.h"

MidiOutputHandler::MidiOutputHandler(QString group, QString key,
MidiOutputHandler::MidiOutputHandler(const QString& group, const QString& key,
MidiController *controller,
float min, float max,
unsigned char status, unsigned char midino,
unsigned char on, unsigned char off)
: m_pController(controller),
m_cobj(ControlObject::getControl(ConfigKey(group, key))),
m_cot(group, key),
m_min(min),
m_max(max),
m_status(status),
m_midino(midino),
m_on(on),
m_off(off),
m_lastVal(0) {
connect(&m_cobj, SIGNAL(valueChanged(double)),
connect(&m_cot, SIGNAL(valueChanged(double)),
this, SLOT(controlChanged(double)));
}

MidiOutputHandler::~MidiOutputHandler() {
ConfigKey cKey = m_cobj.getKey();
ConfigKey cKey = m_cot.getKey();
if (m_pController->debugging()) {
qDebug() << QString("Destroying static MIDI output handler on %1 for %2,%3")
.arg(m_pController->getName(), cKey.group, cKey.item);
}
}

bool MidiOutputHandler::validate() {
return m_cobj.valid();
return m_cot.valid();
}

void MidiOutputHandler::update() {
controlChanged(m_cobj.get());
controlChanged(m_cot.get());
}

void MidiOutputHandler::controlChanged(double value) {
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/midi/midioutputhandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class MidiController; // forward declaration
class MidiOutputHandler : QObject {
Q_OBJECT
public:
MidiOutputHandler(QString group, QString key, MidiController* controller,
MidiOutputHandler(const QString& group, const QString& key, MidiController* controller,
float min, float max,
unsigned char status, unsigned char midino,
unsigned char on, unsigned char off);
Expand All @@ -32,7 +32,7 @@ class MidiOutputHandler : QObject {

private:
MidiController* m_pController;
ControlObjectThread m_cobj;
ControlObjectThread m_cot;
float m_min;
float m_max;
unsigned char m_status;
Expand Down
Loading