Russian Qt Forum
Май 04, 2024, 21:24 *
Добро пожаловать, Гость. Пожалуйста, войдите или зарегистрируйтесь.
Вам не пришло письмо с кодом активации?

Войти
 
  Начало   Форум  WIKI (Вики)FAQ Помощь Поиск Войти Регистрация  

Страниц: 1 ... 31 32 [33] 34 35 ... 88   Вниз
  Печать  
Автор Тема: Создаю библиотеку для работы с последовательными портами. [УШЕЛ ИЗ ПРОЕКТА].  (Прочитано 752967 раз)
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #480 : Июнь 20, 2011, 16:05 »

Цитировать
Название его оставить SerialPortPrivate. А вот конкретные реализации - SerialPortPrivateWin/Unix.
Не, я сделаю как планировал ранее. ИМХО, так красивше.

Цитировать
А разве мьютекс не нужен?
А зачем? В сокетах тоже нет мьютекса.

Цитировать
слово native обязательно надо у каждого метода писать?
Ок, оставлю только у действительно "нативных" методов.

Цитировать
Что за "SerialPortPrivateInfo"? Должен же быть "SerialPortInfoPrivate", вроде. Разве нет?
Ошибся.

Записан

ArchLinux x86_64 / Win10 64 bit
b-s-a
Гость
« Ответ #481 : Июнь 20, 2011, 16:25 »

Цитировать
слово native обязательно надо у каждого метода писать?
Ок, оставлю только у действительно "нативных" методов.
А оно надо? Зачем делается приватный класс - чтобы скрыть детали реализации и для повышения бинарной совместимости между версиями класса. Поэтому, я не вижу ни одной причины для использования слова native.
Записан
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #482 : Июнь 20, 2011, 18:12 »

ok
« Последнее редактирование: Июнь 28, 2011, 14:35 от kuzulis » Записан

ArchLinux x86_64 / Win10 64 bit
lit-uriy
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 3880


Просмотр профиля WWW
« Ответ #483 : Июнь 21, 2011, 17:01 »

Цитата: pastor
2)
Предлагаю работать не напрямую с репозиторием, а посредством мердж-реквестов
А это как? Желательно по пунктам Улыбающийся
Ищи в этой же теме, я как-то тебе делал "Запрос на слияние" ещё в самом начале как ты на гиториусе поселился.
Записан

Юра.
b-s-a
Гость
« Ответ #484 : Июнь 21, 2011, 17:25 »

Зачем нужен метод void AbstractSerialPortPrivate::initVariables()? Чем конструктор не подходит?
Предлагаю вместо этого:
Код
C++ (Qt)
virtual bool setRate(qint32 rate, SerialPort::Directions dir) = 0;
virtual qint32 rate(SerialPort::Directions dir) const = 0;
Для минимизации потенциального дублирования кода, использовать это (AbstractSerialPortPrivate):
Код
C++ (Qt)
bool setRate(qint32 rate, SerialPort::Directions dir) {
  if (setNativeRate(rate, dir)) {
     mRate = rate;
     return true;
  }
  return false;
}
qint32 rate(SerialPort::Directions dir) const {
  if (dir & Input)
     return (dir & Output) ? ((mInRate + mOutRate)/2) : mInRate;
  return mOutRate;
}
//...
protected:
virtual bool setNativeRate(int rate, SerialPort::Directions dir) = 0;
Аналогично с остальными методами (т.е. чтобы не писать в каждой реализации "mParity = parity" и т.п.).
Код
C++ (Qt)
virtual bool saveOldsettings() = 0;
virtual bool restoreOldsettings() = 0;
Действительно ли это нужно? Или все-таки можно использовать стандартные механизмы?

А вот это:
Код
C++ (Qt)
   virtual void detectDefaultRate() = 0;
   virtual void detectDefaultDataBits() = 0;
   virtual void detectDefaultParity() = 0;
   virtual void detectDefaultStopBits() = 0;
   virtual void detectDefaultFlowControl() = 0;
 
   void detectDefaultSettings()
   {
       detectDefaultRate();
       detectDefaultDataBits();
       detectDefaultParity();
       detectDefaultStopBits();
       detectDefaultFlowControl();
   }
я предлагаю упростить до:
Код
C++ (Qt)
virtual void detectDefaultSettings() {}
Так как реализовывать 5 виртуальных функций, которые вызываются только в одном месте я не вижу никакого смысла.

Кстати, названия полей класса будем приводить к стандартному в Qt виду (m_xxxx)?

Файл qserialdevice_global.h имхо не очень нужен, тем более, что он (по идее) должен использоваться в публичных хидерах (а еще у него лицензия GPL). Для начала предлагаю просто ограничиться в каждом публичном хидере:
Код
C++ (Qt)
#ifdef SERIALPORT_SHARED
# ifdef SERIALPORT_BUILD
#  define SERIALPORT_EXPORT Q_DECL_EXPORT
# else
#  define SERIALPORT_EXPORT Q_DECL_IMPORT
# endif
#else
# define SERIALPORT_EXPORT
#endif
Ты так и не сделал разделение serialport_p.h на serialport_p_win.h и serialport_p_unix.h. А стоило бы - на данный момент наличие AbstractSerialPortPrivate смысла не имеет.

Сделал ряд исправлений. см. аттачмент.
Записан
b-s-a
Гость
« Ответ #485 : Июнь 21, 2011, 17:31 »

kuzulis, кстати, очень не рекомендую вызывать виртуальные методы из конструкторов. Особенно, в надежде, что конструктор базового класса вызовет переопределенный наследником метод. По стандарту, виртуальные методы наследника становятся доступны (v_table базового класса замещается на таблицу наследника) непосредственно перед входом в его конструктор.

И размещай уже файлы на гиториусе - работать будет проще.
« Последнее редактирование: Июнь 21, 2011, 17:46 от b-s-a » Записан
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #486 : Июнь 21, 2011, 18:32 »

А как мне склонировать ветку 2.0? А то клонирую мастера постоянно.
Записан

ArchLinux x86_64 / Win10 64 bit
b-s-a
Гость
« Ответ #487 : Июнь 21, 2011, 19:04 »

$ git checkout 2.0
Записан
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #488 : Июнь 21, 2011, 20:36 »

b-s-a
Спасибо, вроде разобрался и залил твою версию.

Цитировать
И размещай уже файлы на гиториусе - работать будет проще.
Смеющийся у меня на работе админом закрыты все порты кроме 8080 и т.п.. , т.е. я не могу оттуда сразу вносить изменения.
Поэтому я аттачу на форум, а потом, из дома уже коммиты вношу.

Цитировать
кстати, очень не рекомендую вызывать виртуальные методы из конструкторов. Особенно, в надежде, что конструктор базового класса вызовет переопределенный наследником метод. По стандарту, виртуальные методы наследника становятся доступны (v_table базового класса замещается на таблицу наследника) непосредственно перед входом в его конструктор.
Ок.

Цитата: b-s-a
Зачем нужен метод void AbstractSerialPortPrivate::initVariables()? Чем конструктор не подходит?
Подходит. Просто писанины в конструкторе прибавится.

Цитата: b-s-a
Предлагаю вместо этого:
Код
C++ (Qt)
virtual bool setRate(qint32 rate, SerialPort::Directions dir) = 0;
virtual qint32 rate(SerialPort::Directions dir) const = 0;
 
...
...
...
Ок. Только нужно наверное немного изменить твою реализацию методов setRate() / rate().

Цитата: b-s-a
Аналогично с остальными методами
Ну, вот, с методами setPort() / port() так просто не получится. Т.е. твою реализацию нужно подправить.
Что имею ввиду:
переменная mPort должна по идее всегда хранить длинные имена портов,
нативный метод setPort() (к примеру, для винды) должен из короткого имени "COM1" делать длинное "\\\\.\\COM1", а если на входе длинное имя - то ничего не делать.
И далее, это длинное имя он присваивает переменной mPort. (В линуксе и пр. ОС аналогично).
Метод же port() должен из длинного имени делать короткое и возвращать результат (мы ж так условились).

Поэтому можно было бы добавить еще методы nativeSetPort() / nativePort(). Т.е. для каждой из платформ будет своя реализация (на QRegExp к примеру).
Но если оставить так как есть сейчас - то придется химичить с дефайнами в serialport_p.h, что не есть хорошо, т.к. для этого есть serialport_p_unix/win.h
ИМХО.

Цитата: b-s-a
Код
C++ (Qt)
virtual bool saveOldsettings() = 0;
virtual bool restoreOldsettings() = 0;
 
Действительно ли это нужно? Или все-таки можно использовать стандартные механизмы?
Что имеешь ввиду под стандартными механизмами?
Сохранять/восстанавливать предыдущие параметры то нужно по-любому (после открытия и перед закрытием).

Насчет упростить и оставить virtual void detectDefaultSettings() {} :
ну, тут если упрощать - то слишком много кода придется писать в этот метод,
т.е. будет громоздко. ИМХО, лучше его разбить на 5 стадий по смыслу.

Насчет префиксов m/m_ у приватных членов - то может быть, их вообще не писать (т.е. опускать их)?
т.к. я бегло просмотрел исходники Qt и там очень редко встречается этот префикс.
Хотя, в исходниках QtCreator - повсеместно.
А то лишняя писанина получается и имя переменной растет.

----
А в целом, просмотрев твои идеи, согласен, давай так.

Цитата: lit-uriy
Ищи в этой же теме, я как-то тебе делал "Запрос на слияние" ещё в самом начале как ты на гиториусе поселился.
Улыбающийся ок, думаю по ходу процесса, думаю, мне станет яснее.
« Последнее редактирование: Июнь 28, 2011, 14:35 от kuzulis » Записан

ArchLinux x86_64 / Win10 64 bit
b-s-a
Гость
« Ответ #489 : Июнь 21, 2011, 22:28 »

Ну, вот, с методами setPort() / port() так просто не получится. Т.е. твою реализацию нужно подправить.
Что имею ввиду:
переменная mPort должна по идее всегда хранить длинные имена портов,
нативный метод setPort() (к примеру, для винды) должен из короткого имени "COM1" делать длинное "\\\\.\\COM1", а если на входе длинное имя - то ничего не делать.
И далее, это длинное имя он присваивает переменной mPort. (В линуксе и пр. ОС аналогично).
Метод же port() должен из длинного имени делать короткое и возвращать результат (мы ж так условились).

Поэтому можно было бы добавить еще методы nativeSetPort() / nativePort(). Т.е. для каждой из платформ будет своя реализация (на QRegExp к примеру).
Но если оставить так как есть сейчас - то придется химичить с дефайнами в serialport_p.h, что не есть хорошо, т.к. для этого есть serialport_p_unix/win.h
ИМХО.
Тогда уж лучше сделать методы:
Код
C++ (Qt)
virtual QString portName() const = 0; //возвращает короткое имя
virtual bool setNativePort(const QString &port) = 0; //устанавливает полный путь порта

Цитата: b-s-a
Код
C++ (Qt)
virtual bool saveOldsettings() = 0;
virtual bool restoreOldsettings() = 0;
 
Действительно ли это нужно? Или все-таки можно использовать стандартные механизмы?
Что имеешь ввиду под стандартными механизмами?
Сохранять/восстанавливать предыдущие параметры то нужно по-любому (после открытия и перед закрытием).
Не спорю о необходимости восстановления (хотя, я бы сделал возможность отключения этой возможности через некое свойство), я просто предложил восстановить через setParity/setRate/...
Насчет упростить и оставить virtual void detectDefaultSettings() {} :
ну, тут если упрощать - то слишком много кода придется писать в этот метод,
т.е. будет громоздко. ИМХО, лучше его разбить на 5 стадий по смыслу.
Разбей хоть на 50. Но это же можно сделать статическими функциями внутри cpp или приватными функциями классов-реализаций.
Насчет префиксов m/m_ у приватных членов - то может быть, их вообще не писать (т.е. опускать их)?
т.к. я бегло просмотрел исходники Qt и там очень редко встречается этот префикс.
Хотя, в исходниках QtCreator - повсеместно.
А то лишняя писанина получается и имя переменной растет.
Я лично использую суффикс _: rate_, parity_, ...
Записан
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #490 : Июнь 22, 2011, 18:16 »

b-s-a,

Я пока заккомитил свою версию.

После этого увидел, что и ты мерж новый подогнал.

Вопросы:
1. Почему ты сменил каталог src на lib?
2. Зачем ты в serialport.cpp/serialportinfo.cpp
добавил дефайны и подключаешь приватные заголовки?
Код
C++ (Qt)
#ifdef Q_OS_WINDOWS
# include "serialport_p_win.h"
#elif defined(Q_OS_UNIX)
# include "serialport_p_unix.h"
#else
# error Unsupported Operating System
#endif
 
Имхо, это не правильно. Реализация в serialport.cpp ничего не должна знать об платформо зависимых вещах.
3. Почему убрал *.pri файл?

« Последнее редактирование: Июнь 22, 2011, 18:29 от kuzulis » Записан

ArchLinux x86_64 / Win10 64 bit
pastor
Administrator
Джедай : наставник для всех
*****
Offline Offline

Сообщений: 2901



Просмотр профиля WWW
« Ответ #491 : Июнь 22, 2011, 18:31 »

Заглянул в исходники на гиториусе (serialport_p_win.cpp):

Код
C++ (Qt)
bool SerialPortPrivateWin::dtr() const
{
   return (SerialPort::Dtr && lines());
}
 
bool SerialPortPrivateWin::rts() const
{
   return (SerialPort::Rts && lines());
}

Нужно бы исправить && на &
Записан

Integrated Computer Solutions, Inc. (ICS)
http://www.ics.com/
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #492 : Июнь 22, 2011, 18:52 »

Цитата: pastor
Нужно бы исправить && на &
Тьфу, заработался. Там еще много косяков.
Повелся не думая за b-s-a Улыбающийся

2 b-s-a,
просмотрел еще раз твой мерж, и не понял: а чем тебя не устроил мой первоначальный вариант с AbstractPortPrivate ?
Тем более, что в твоей реализации нужно дополнительно создавать два лишних файла serialport_p_win/unix.h (зачем???), в которых нужно дублировать методы и т.п.
Я то думал, это какой-то особый "финт", но преимуществ такого разделения так и не увидел., только лишние телодвижения, ИМХО.
Я склоняюсь больше к варианту аналогичному как в сокетах.
« Последнее редактирование: Июнь 22, 2011, 19:01 от kuzulis » Записан

ArchLinux x86_64 / Win10 64 bit
pastor
Administrator
Джедай : наставник для всех
*****
Offline Offline

Сообщений: 2901



Просмотр профиля WWW
« Ответ #493 : Июнь 22, 2011, 19:26 »

Я склоняюсь к такому "набору" файлов:

serialport_p.h
serialport_win_p.cpp
serialport_mac_p.cpp
serialport_unix_p.cpp

Аналогично для приватного класса serialportinfo
Записан

Integrated Computer Solutions, Inc. (ICS)
http://www.ics.com/
kuzulis
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2812


Просмотр профиля
« Ответ #494 : Июнь 23, 2011, 07:17 »

pastor ,
да и я тоже к такому.
Записан

ArchLinux x86_64 / Win10 64 bit
Страниц: 1 ... 31 32 [33] 34 35 ... 88   Вверх
  Печать  
 
Перейти в:  


Страница сгенерирована за 0.32 секунд. Запросов: 23.