Как стать автором
Обновить

Комментарии 47

Трам-парарам. MVP хорош, когда у вас есть набор данных, у вас его нет, он формируется на ходу независимым образом от пользователя. В вашем случае, когда взаимодействие между моделью и гуи велико лучше пойдет MVC
Я с вами вполне согласен. Тот пример, что привел я, проще реализовать, используя MVC. Когда я брался за свой проект, взвесив все за и против MVC и MVP, выбрал MVP. Реальный проект сложнее, чем то, который приведен в примере, поэтому явно, почему был выбран именно MVP, возможно не видно. Но и цели такой не было, была цель показать подход.
По-моему метод run() лучше объявить как protected, т.к. он у класса QThread является virtual protected:

void QThread::​run()
Вы абсолютно правы, так будет наиболее правильно!
А разве в таком виде при отправки в ком порт данных из гуя он не подвиснет? Вообще, какой то очень не Qtшный подход к решению.
А почему он должен подвисать?!
Наиболее времязатратное действие происходит в другом потоке, влияние на GUI быть не должно. Во всяком случае данный подход какого-либо заметного влияния на работу GUI не оказывает.
Но это просто потому, что эти методы быстро выполняются, а может так и не повезти. По хорошему нужно создать класс порта, который будет общаться с собственно ком портом путем обменом асинхронных сообщений, а иначе какой вообще смысл в тредах- если мы то и дело из одного треда вызываем методы объекта, принадлежащего другому. Такого вообще по хорошему нужно избегать.
Здесь я склонен с вами не согласится. Есть два подхода, я об этом упомянул в тексте статьи.
Можно помещать объект в поток, используя QObject::moveToThread, а можно наследоваться от QThread.
Сами разработчики-Qt используют их оба в своей документации и примерах.
Я в своей работе использовал и тот и другой подход. У каждого есть свои плюсы и минусы.

Если не сложно, хотелось бы увидеть какие-либо реальные примеры, показывающие, что использование наследования от QThread — это плохо.
Спасибо за статью, как-то прошла мимо меня, но в ней опять же не раскрывается тема, почему наследование от QThread — это плохо и к чему оно может привести.
Было бы интересно уточнить именно этот момент, чтоб окончательно поставить все точки на i.
QThread следует рассматривать не как сам поток, а как некую надстройку, предоставляющую доступ к потоку. На практике это приводит к тому, что в отдельном потоке будет работать только код, находящийся в функции run(), или вызываемый непосредственно из нее. Вот, например, такой кусок кода:

class Thread : public QThread
{
    Q_OBJECT
private slots:
    void onTimeout()
    {
        qDebug() << "Thread::onTimeout вызван из потока: " << QThread::currentThreadId();
    }

private:
    void run()
    {
        qDebug() << "Поток-работник:" << currentThreadId();
        QTimer timer;
        connect(&timer, SIGNAL(timeout()), this, SLOT(onTimeout()));
        timer.start(1000);

        exec();
    }
};


Казалось бы, все здорово — таймер создается и запускается в отдельном потоке, и подключается к слоту наследника от QThread'а, то есть вроде как и onTimeout тоже должен быть вызван в потоке-работнике, но это не так. В большинстве простых случаев, подход с переопределением run() действительно будет работать, но если вам нужна какая-то сложная логика обмена сообщениями, или синхронизация потоков, то могут возникнуть сложноотлаживаемые проблемы.

Вот здесь (вторая часть) есть более подробная статья с несколькими примерами «правильно/неправильно».
Отличные ссылки, спасибо! Как-то прошли мимо меня.
Если вы не против, добавлю их в статью.

Тот пример, который привели вы, немного отличается от того, что привел я, хотя бы потому, что в вашем примере ожидается, что слот будет обрабатываться в том же потоке, что и сигнал. А это не так. Это вводит в заблуждение.

В моем же примере вся работа по отправке и приему будет проходить в одном потоке, а вот обработка сигналов будет в другом потоке.
Если подытожить, то в двух словах можно сказать так, все, что вы поместите в метод run(), будет выполняться в этом потоке, во всех остальных случаях в основном потоке, за исключением случая, когда присоединение слотов и сигналов происходит в методе run().
«Если подытожить, то в двух словах можно сказать так, все, что вы поместите в метод run(), будет выполняться в этом потоке, во всех остальных случаях в основном потоке, за исключением случая, когда присоединение слотов и сигналов происходит в методе run().»

Это не совсем так:
connect(&timer, SIGNAL(timeout()), this, SLOT(onTimeout()));
в коде выше соединение идет как раз в run(), только слот выполняется не в рабочем потоке.
Да, вы правы, не совсем верно выразился, обработка сигнала в рабочем потоке будет в случае такого использования:
run()
{
    ....

    Object a;
    ....

    connect(&timer, SIGNAL(timeout()), &a, SLOT(onTimeout()));
    ....
}
нет, вы снова не правы. Код

Object *a;
run()
{
    ....
 a = new Object();
   
}
где-то после во внешнем коде
connect(&timer, SIGNAL(timeout()), &a, SLOT(onTimeout()));

также будет работать как надо.
После вызова connect, QObject определяет к какому потоку относятся эммитер и приемник и производит связывание.
Любой класс, наследуемый от Qobject при вызове конструктора определяет свое место положение через статическую функцию QThread::currentThread. Это местоположение можно поменять через moveToThread.
Переподключатся при этом сигналы — этим вопросом задался только сейчас.

Ничего там не переподключается. Дело в том, что у QObject::connect() есть пятый аргумент: тип соединения. По умолчанию там Qt::AutoConnection. Что делается при выстреле сигнала, определяется в рантайме по типу соединения. Вот в этом месте.
Я имел ввиду именно ситуацию когда мы вызвали коннект, а потом сказали moveToThread. Что будет при этом. Теперь понятно.
Тут стоит еще сказать что заводить event loop (делать exec()) для Timer в рабочем потоке вообще не лучшее решение.
QThread так использовать нельзя, у вас сигналы вызываются на прямую, что приведёт к многопоточным багам, взаимоблокировкам потоков и прочим радостям, у вас всё не падает только из за везения и очевидно простой логики в гуёвом потоке.
Я не совсем уверен что в данном случае есть прямая зависимость от логики в гуёвом потоке и стабильностью работы. Может приведете пример, что мне надо изменить, чтобы программа упала?
Какой-то C-Style чтение из порта.
readyRead() — сигнал, который вызывается когда на QIODeivce появляются данные. Никаких «while» true не надо. Просто помещаете QSerialPort в поток и ждете readyRead.
Тут есть один нюанс, дело в том, что Windows не система реального времени. А это значит, что посылаете вы допустим с устройства 18 байт. В COM-порт он попадают побайтно, сама система их может выдать сразу 18, или сначала 10, потом 8. Или сначала 12, потом 6.
Что мы получим в этом случае, принимаем сигнал readyRead, вызываем readAll и получаем 12 байт вместо 18. Потому что в момент прихода сигнала в буфер успело попасть только 12 байт, еще 6 придут чуть позже. Проверено не единожды с использованием разных контроллеров и программы AdvansedSerialPort, запущенной в режиме сниффера.
while((buffer=readAll()).size()) по readyRead() не?
Как вариант можно и так использовать.
Надо будет протестировать на реальном железе.
Добавлю еще, способ, приведенный выше не подходит по двум причинам, во-первых, нужно всегда знать длину пакета. Во-вторых, если устройство не отвечает на запрос, мы об этом не узнаем, будем крутиться в потоке, в ожидании пакета.
А если не 6 придут, а 8 или 9? Если по другому пакеты порежутся. По хорошему нужна двойная буферизация.
Если придет больше (с чем сталкиваться еще не приходилось ни разу, но вдруг) ничего страшного. При разборе пакета этот нюанс учтен, вместе с пакетом передается его длина и контрольная сумма.
Вместо двойной буферизации можно использовать кольцевой буфер, тогда мы ничего не пропустим.
Касательно вот этого:
QSerialPort serial;
// Блокируем поток для инициализации переменных потока
mutex.lock();
// Имя текущего COM-порта
QString currentPortName = m_portName;

// Время ожидания ответа
int currentWaitTimeout = m_waitTimeout;

// Информация, отправляемая в COM-порт
QByteArray currentRequest = m_request;
mutex.unlock();

Зачем блокировать поток на инициализации локальных переменных, даже не обращающихся ко внешним устройствам?
// Открываем COM-порт
if (serial.open(QIODevice::ReadWrite))
{

А вот тут как раз блокировка не помешала бы, хотя я могу и ошибаться и ядро ОС само разрулит множественный доступ.
В целом вы как раз явно продемонстрировали почему данный подход к потокам плох. Инициализацию ресурсов лучше производить в одном потоке. А вот их использование уже хэндлить в нескольких.
Для уверенности, что в процессе инициализации значения переменных не будут изменены извне. Возможно это лишнее, такие действия основываются на тех примерах, которые идут с Qt.
При открытии COM-порта это будет лишнее, ИМХО.
Инициализацию ресурсов мне кажется не очень правильно делать в одном потоке, а использование в другом, кто даст гарантию в таком случае, что на середине передачи значения, в массиве, который передается, не изменятся из другого потока?!
Не получится ли так, что мне нужно передать последовательность байт (0x20, 0x45, 0x12, 0x17), но в момент передачи значение третьего байта внешний поток поменяет на 0x14, и у меня в COM-порт уйдет (0x20, 0x45, 0x14, 0x17)?
Для уверенности, что в процессе инициализации значения переменных не будут изменены извне. Возможно это лишнее, такие действия основываются на тех примерах, которые идут с Qt.


Инициализацию ресурсов мне кажется не очень правильно делать в одном потоке, а использование в другом, кто даст гарантию в таком случае, что на середине передачи значения, в массиве, который передается, не изменятся из другого потока?!

Я конечно сильно надеюсь, что это не так, но по-моему Вы не понимаете как работают потоки.
Ну что ж, предлагаю обратиться к документации.
Пример:
void MasterThread::transaction(const QString &portName, int waitTimeout, const QString &request)
{
    QMutexLocker locker(&mutex);
    this->portName = portName;
    this->waitTimeout = waitTimeout;
    this->request = request;
   ....
}

Метод transaction вызывается из основного потока.
Пояснение:
«Note, the transaction() method is called in the main thread, but the request is provided in the MasterThread thread. The MasterThread data members are read and written concurrently in different threads, thus the QMutex class is used to synchronize the access.»

И далее:
«The transaction() method stores the serial port name, timeout and request data. The mutex can be locked with QMutexLocker to protect this data. The thread can be started then, unless it is already running.»

Возможно я чего-то не так понял?
Отвечу сам себе, вы абсолютно правы, в том контексте, который привели вы, блокировка не нужна. Не сразу понял о какой части кода идет речь. Прошу прощения и посыпаю голову пеплом.
if (!m_instance)
{
mutex.lock();

if (!m_instance)
{
m_instance = new ComPortThread;
}

m_refCount++;
mutex.unlock();
}

Поясните пожалуйста чем обусловлена повторная проверка m_instance?
При входе в функцию первого потока он проверяет проинициализирован ли указатель.
Если да возвращаем текущее значение, все ок.

Если нет то захватываем мьютекс для инициализации. Если мьютекс уже захвачет это значит то инициализация началась в другом потоке.
Этот поток висит на локе. после того как поток который успел войти в функцию ранее освобождает мьютекс, текущий поток двигается дальше, захватывает мьютекс, и нам нужно убедится что m_instance все еще не проинициализирован. Ну и дальлше если все ок возвращаем текущее значение указателя.
В текущей реализации есть ошибка:
m_refCount не будет инкрементирован если m_instance проинициализирован на момент входа.

А вообще для синглтонов я недавно открыл для себя отличную реализацию на атомарных типах. В целом она не претендует на гениальность и я думаю, что ее придумали задолго для меня, но намного лаконичнее и удобнее. Правда есть один нюанс она работает не на всех архитектурах. Хотя и в этом нет полной уверенности, потому что я не проверял. Но на ARM x86 amd64 все ок.
«А вообще для синглтонов я недавно открыл для себя отличную реализацию на атомарных типах»
А где можно почитать об этом?
Не читал перед реализацией, но статья именно об этом. Кстати надо бы перевести и выложить на хабр. Статья действительно отличная!
Вообще не понял причем тут Qt? Никаких прелестей Qt почти не используется, вообще всё никак не в стиле Qt. Легче былоб вообще от него отказаться, если не важна кроссплатформенность. Может у вас просто не было толком опыта работы с Qt?
Можно у вас попросить пример программы(а лучше статью) полного использования прелестей Qt в стиле Qt?
Я думаю мне, да и не только мне, будет полезно поучиться у вас, как надо использовать Qt.
Ни в коем случае не хотел вас обидеть, обвинять в невежестве или восхвалять себя. Тем более я себя вообще не считаю хорошим Qtшником. А хороший пример — сами исходники Qt.
Я просто высказал мнение, что на вашем месте я бы выбрал другой более легкий и удобный инструмент. А " полного использования прелестей Qt в стиле Qt" даже и не нужно. Простите, если ввел вас в заблуждение, я просто тяжело высказываю свои мысли.
Я бы выделил всю работу с портом в отдельный класс, который бы информировал и получал команды через сигналы и слоты, затем в run методе создал бы объект этого класса, соединил бы сигналы и слоты с внешним классом, что то типа connect(&socket, SIGNAL(signalDataReceived(QByteArray)),
SLOT(listen(QByteArray)), Qt::QueuedConnection); и запустил бы эвент луп — exec().
Немного повторюсь:" данная реализация не претендует на абсолютно правильную, это скорее описание, одного из вариантов, как можно сделать."
Я считаю, что не ошибается только тот, кто ничего не делает.
Эта статья появилась, потому что было желание рассказать и показать, как можно сделать. Возможно в будущем она кому-то пригодится, может кто-то найдет в ней что-то полезное для себя, какие-то идеи или возможности в реализации своей задумке.
Она не ставит своей целью научить кого-то, как надо программировать. Но, я лично считаю, что чем больше подобных статей будет на русском языке, тем больше у разработчиков будет информации о той или иной технологии, тем легче им будет работать.
В конце концов, по мере обсуждения всплыли кое-какие подробности реализации, лично которых я не знал. Кто-то прочитает, взвесит для себя все за и против, и сделает выбор в сторону того или иного способа работы.
Если это будет действительно так, значит я не зря потратил свое время на написание данной статьи, а ваше время на ее прочтение.
Вопрос не в том что зря, а что не зря. Я не задаюсь целью писать статьи по ставшим для меня обыденным аспектам Qt(может и зря), потому что нахожу документацию Qt избыточной. Просто критика во многом обоснована, из-за объективных ошибок в использовании Qt. Я понимаю стремление популяризировать Qt и это похвально, но ошибки работы с потоками например моментально отталкивают любого программиста от использования вашего решения. Опять же простота использования Qt не раскрыта из-за громоздкого чтения(на мой взгляд).

Все ведут к тому, что возможно Вы сами не слишком хорошо разобрались в теме, для написания этой статьи, и в следующий раз следует поработать над этим получше.
Не стоит так категорично, у человека меньше опыта чем у вас, но человек старался.
У всех бывают ошибки, а статей мало, к тому же ошибки были поправлены вами и другими экспертами, что сделало в итоге статью лучше, а так статьи может быть не было вообще.
Может случайный прохожий получить exe-файл просто для ознакомления?
Presenter – это связующее звено между View и Model.

Может, Proxy[Link] – это связующее звено между View и Model. :)
Можно избавиться от цикла while() в методе QThread::run(), используя QStateMachine. Заодно, получить расширенные возможности создания точек выхода из рабочего потока по n-количеству условий.
Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Изменить настройки темы

Истории