QDial and wrapping

SGaist

Hi,

Might be a sill question but isn't the wrapping property what you want to implement ?

admkrk

Hi,

Thanks for your reply. Yes, but as I mentioned the value resets when it wraps. For example in the range I have set after 11 it jumps to 1, or the other way when decreasing. That is what was causing the jump. I tried adding a function to just determine the direction.

int MainWindow::dialDirection(int value)
{
    // should only happen at startup
    if(previousValue == -1)
    {
        previousValue = value;
        return 3;
    }

    if(previousValue < value)
    {
        if(value == 1 && previousValue == 11)
            // Dial rolled over, still increasing
            return 1;
    }

    if(previousValue > value)
    {
        if(value == 11 && previousValue == 1)
            // Dial rolled over, still decreasing
            return -1;
    }

    previousValue = value;

    return 0;
}

That seems to be doing the trick with a little change in the other function.

void MainWindow::on_dialTime_valueChanged(int value)
{
    direction = dialDirection(value);

    if(setHrs)
    {
        if(direction == 1/*previousHour < value*/)
            ++curHrs;
        else
            --curHrs;

        curHrs = calculateHours(curHrs);
        ui->timeHours->display(curHrs);
    }

I have not really tested it to see if it works other than for the hours, but it will eliminate a few variables if it does. Using it directly with curHrs does not seem to work:

curHrs += direction;

I am not really surprised, that would have been too easy. Need to look and see what is happening, but if I can eliminate the if/else also I will be happy.

Pablo J. Rogina

@admkrk this is my approach, trying to split the rotary encoder value changed signal from the use of such event, I mean, from using the encoder increasing or decreasing.
I created a QDialog with a QDial, radio buttons for selecting hours or minutes to set and a QLCDNumber (simplified to just one LCD, you'll figure out)

dialog.h

class Dialog : public QDialog
{
    Q_OBJECT

public:
    explicit Dialog(QWidget *parent = nullptr);
    ~Dialog();

    enum EncoderDirection {
      Increase = 1,
      Decrease = -1
    };
    Q_ENUM(EncoderDirection)

signals:
    void encoderValueChanged(EncoderDirection direction);

private slots:
    void on_dial_valueChanged(int value);
    void updateLCD(EncoderDirection direction);

private:
    Ui::Dialog *ui;
    int currentValue;
};

dialog.cpp

Dialog::Dialog(QWidget *parent) :
    QDialog(parent),
    ui(new Ui::Dialog)
{
    ui->setupUi(this);
    currentValue = ui->dial->value();
    ui->dial->setFocus();
    connect(this, &Dialog::encoderValueChanged, this, &Dialog::updateLCD);
}

Dialog::~Dialog()
{
    delete ui;
}

void Dialog::on_dial_valueChanged(int value)
{
    EncoderDirection encoderDirection;
    if (value > currentValue) {
        if (currentValue == ui->dial->minimum()) {
            encoderDirection = EncoderDirection::Decrease;
        } else {
            encoderDirection = EncoderDirection::Increase;
        }
    } else {
        if (currentValue == ui->dial->maximum()) {
            encoderDirection = EncoderDirection::Increase;
        } else {
            encoderDirection = EncoderDirection::Decrease;
        }
    }
    currentValue = value;
    emit encoderValueChanged(encoderDirection);
}

void Dialog::updateLCD(EncoderDirection direction) {
    if (ui->radioHour->isChecked()) {
        int hour = ui->lcdNumber->intValue() + direction;
        if (hour > 12) {
            hour = 1;
        } else if(hour < 1) {
            hour = 12;
        }
        ui->lcdNumber->display(hour);
    }
// ... if radio button is set for minutes, update that display etc...
}

admkrk

Hi Pablo, that is a nice clean example and shows me a couple ways I can clean my code up.

Obviously my previous solution did not work. One problem I was having is that the values are different when rolling over one direction compared to the other. It still needs cleaned up, but I got it working with this.

int MainWindow::dialDirection(int value)
{
    // should only happen at startup
    if(previousValue == -1)
    {
        previousValue = value;
        return 3;
    }

    if(previousValue > value)
    {
        if(value == 1 && previousValue == 11)
        {
            // Dial rolled over, so increasing
            previousValue = value;
            return 1;
        }
        else
        {
            // Decreasing
            previousValue = value;
            return -1;
        }
    }

    if(previousValue < value)
    {
        if(value == 12 && previousValue == 2)
        {
            // Dial rolled over, so decreasing
            previousValue = value;
            return -1;
        }
        else
        {
            // Increasing
            previousValue = value;
            return 1;
        }
    }

    // Something bad happened
    return 0;
}

And setting the time.

void MainWindow::on_dialTime_valueChanged(int value)
{
    direction = dialDirection(value);

    if(setHrs)
    {
        curHrs += direction;
        curHrs = calculateHours(curHrs);
        ui->timeHours->display(curHrs);
    }

Thank you very much.

I am marking this solved, but I noticed I am getting this in the debug output.

QMetaObject::connectSlotsByName: No matching signal for on_dial_clicked()
QObject::connect: No such signal Dial::valueChanged() in ..\MistingGui\mainwindow.cpp:36
QObject::connect:  (sender name:   'dialTime')
QObject::connect:  (receiver name: 'MainWindow')

It is working, so it cannot be an error. Could it just be that I am using the old style of connect? Or is something I did wrong subclassing QDial?

Pablo J. Rogina

@admkrk said in QDial and wrapping:

Or is something I did wrong subclassing QDial?
you should share the code of your subclass. It's hard to say without being able to see it.

It is working, so it cannot be an error.

Definitely it is not good :-) Such message should not appear at all

admkrk

Hi Pablo,

I really did not do much with the subclass, just made it so it would ignore clicks. Clicking it sets which part of the time to change, like the radio button in your example.
In mainWindow.cpp

connect(ui->dialTime, SIGNAL(valueChanged()), this, SLOT(on_dialTime_valueChanged()));

The beggining of on_dialTime_valueChanged() is in my previous post.

#ifndef DIAL_H
#define DIAL_H

#include <QObject>
#include <qdial.h>

class Dial : public QDial
{
    Q_OBJECT
public:
    Dial(QWidget *parent = nullptr);

signals:
    void clicked();

protected:
    void mousePressEvent(QMouseEvent *me) override;
    void mouseReleaseEvent(QMouseEvent *me) override;
    void mouseMoveEvent(QMouseEvent *me) override;
};

#endif // DIAL_H

#include "dial.h"

#include <QMouseEvent>

Dial::Dial(QWidget *parent)
{
    Q_UNUSED(parent)
}

void Dial::mousePressEvent(QMouseEvent *me)
{
    me->ignore();
    emit clicked();
}

void Dial::mouseReleaseEvent(QMouseEvent *me)
{
    me->ignore();
}

void Dial::mouseMoveEvent(QMouseEvent *me)
{
    me->ignore();
}

Pablo J. Rogina

@admkrk said in QDial and wrapping:

connect(ui->dialTime, SIGNAL(valueChanged()), this, SLOT(on_dialTime_valueChanged()));

Two things here:

Please use the new syntax for signals & slots, that way you receive compile-error messages regarding something wrong with connecting signals to slots
Since you're naming your slot as "on_<qt-widget-name>_<signal-name> I guess Qt is using the auto-connection feature to already connect such signal (i.e. valueChanged) to this slot, so you ended up with double connection.

admkrk

I think I got the new syntax figured out, those messages went away. However, I now get a new one

QMetaObject::connectSlotsByName: No matching signal for on_dial_clicked()

I thought it might be the syntax issue again, but changing that one did not make a difference this time. I now have

connect(ui->dialTime, &Dial::clicked, this, &MainWindow::on_dial_clicked);
connect(ui->dialTime, &QDial::valueChanged, this, &MainWindow::on_dialTime_valueChanged);

Changing on_dial_clicked() to on_dialTime_clicked() got rid of the message, but now it does not work.

void MainWindow::on_dialTime_clicked()
{
    ++timeSet;
    if(timeSet > 3)
        timeSet = 0;

    switch(timeSet)
    {
    //...
    }
}

Since timeSet is cycling between 0 and 2, judging from the results, I assume I am getting the double connection you mentioned.

As one last test, I changed the name to dialClicked(). That got rid of the message and it still functioned properly. I am unsure what to make of all that.

Pablo J. Rogina

@admkrk said in QDial and wrapping:

connect(ui->dialTime, &QDial::valueChanged, this, &MainWindow::on_dialTime_valueChanged);

if you keep naming your slots as on_<widget-name>_<signal-name> please don't also use that explicit connection

admkrk

Hi Pablo,

I am still a little confused. It is sort of a habit for me to name my slots on_<random-name>_<signal-name>, where random-name is something meaningful to me. In this case I have four other dials that need the same functionality, so using widget-name would not necessarily be the best solution to begin with.

@Pablo-J-Rogina said in QDial and wrapping:

if you keep naming your slots as on_<widget-name>_<signal-name> please don't also use that explicit connection

That sounds like I do not even need to have the connect() statement in the first place if I use that naming convention. I have mostly been writing code for 8-bit micro-controllers lately, so maybe I am being extra thick headed about this.

Pablo J. Rogina

@admkrk said in QDial and wrapping:

That sounds like I do not even need to have the connect() statement in the first place if I use that naming convention

That's right. I guess there's no way to avoid the "autoconnection" feature when the uic tool generates C++ code from the .ui file, see this bug (open since 2012-11-07...).

So if you feel comfortable naming slots like on_<widget-name>_<signal-name> keep in mind to avoid doing the explicit connect as well.

As further reference, this is the method in QMetaObject class responsible for th autoconnection feature.

admkrk

Thank you Pablo, that cleared up my confusion. I can verify that my other connect() statement was causing a double connection also since if works fine after removing it. I will definitely keep this in mind for the future.

Thanks again, Kirk