Fixed a bunch of compiler warnings and deprecated functions. by sfjuocekr · Pull Request #446 · EdgeTX/edgetx

sfjuocekr · 2021-07-14T02:32:21Z

Fixed a bunch of compiler warnings.
Fixed a bunch of deprecated functions.
Changed the setup script to allow builds on 21.04.

raphaelcoeffic · 2021-07-14T16:49:33Z

Just an idea, but I think we should make some CI/CD script to compile companion when changes to companion are committed, don’t you think?

sfjuocekr · 2021-07-16T18:44:26Z

Probably, but my experience regarding CI/CD is very limited. I'm one of those text editor programmers :(

I've been trying to get my head around QTCreator the past days.

elecpower · 2021-08-01T00:17:19Z

  if (Boards::getFourCC(board) != fourcc) {
    if (IS_FAMILY_HORUS_OR_T16(board) && fourcc == 0x3178396F) {
-      qDebug() << QString().sprintf("%s: Deprecated fourcc used %x vs %x", getName(), fourcc, Boards::getFourCC(board));
+      qDebug() << QString().asprintf("%s: Deprecated fourcc used %x vs %x", getName(), fourcc, Boards::getFourCC(board));


As per QT doco, even asprintf is not recommended for new code so if updating it is recommended to use QTextStream or arg

Support - simplest change

elecpower · 2021-08-01T00:18:11Z

    QDir dir(path);
    if (dir.exists() && dir.isReadable()) {
-      QStringList fwid = getCurrentFirmware()->getId().split("-", QString::SkipEmptyParts);
+      QStringList fwid = getCurrentFirmware()->getId().split("-", Qt::KeepEmptyParts);


Why the change from skip to keep?

Copy paste error!

Needs Qt 5.14 - change not supported

elecpower · 2021-08-01T00:22:10Z

  connect(ui->multiProtocol, static_cast<void(QComboBox::*)(int)>(&QComboBox::currentIndexChanged), this, &ModulePanel::onMultiProtocolChanged);
  connect(this, &ModulePanel::channelsRangeChanged, this, &ModulePanel::setupFailsafes);
-  connect(ui->btnGrpValueType, static_cast<void(QButtonGroup::*)(int)>(&QButtonGroup::buttonClicked), this, &ModulePanel::onFailsafesDisplayValueTypeChanged);
+  connect(ui->btnGrpValueType, static_cast<void(QButtonGroup::*)(int)>(&QButtonGroup::idClicked), this, &ModulePanel::onFailsafesDisplayValueTypeChanged);


Introduced at 5.15 so backwards compatibility issue with OpenTX and buttonClicked is still supported

elecpower · 2021-08-01T00:40:27Z

IMHO there needs to be agreement on what the build environment is otherwise devs will be chasing their tails trying to diagnose why things work or don't considering devs are using Windows, flavours of Linux and maybe even Mac.
I have no problem in devs using the environment of their choice but all PRs should be compiled and tested against the agreed environment.
There are lots of changes to the project code without adding moving platform. This is not to say the platform should stay fixed but maybe review and if necessary update after each major project release.

pfeerick · 2021-08-01T01:08:39Z

Agreed. Just like with the firmware - a set environment (or environments, if it needs to be natively built) - needs to be used for consistency and repeatability. Or at least version locks - i.e. QT 5.12, not just whatever happens to be the latest.

raphaelcoeffic · 2021-08-01T06:16:56Z

Agreed. Just like with the firmware - a set environment (or environments, if it needs to be natively built) - needs to be used for consistency and repeatability. Or at least version locks - i.e. QT 5.12, not just whatever happens to be the latest.

Traditionally, this is done by having the CI/CD using the oldest version supported. Everything must be checked against that version. So bottom line is: we need CI/CD for Companion if we want to be able to do this properly. @elecpower is that what you had in mind?

sfjuocekr · 2021-08-01T17:11:57Z

Some of these changes could be rewritten with #ifdef to check what version is used to silence the compiler on those versions.

elecpower · 2021-08-01T21:11:19Z

I agree with, where practical, suppressing compiler warnings but please no version #ifdef just to suppress compiler warnings. Set the build platform and live with any annoyances.

elecpower · 2021-08-01T21:50:53Z

Traditionally, this is done by having the CI/CD using the oldest version supported. Everything must be checked against that version. So bottom line is: we need CI/CD for Companion if we want to be able to do this properly. @elecpower is that what you had in mind?

@raphaelcoeffic not quite that far but why not have that as longer term aim. CI tests would need to be across supported OS platforms. The real challenge are those obscure issues that arise between OS platforms eg UI display.

Anyway my comment was mainly to set the project's dev environment and all devs code to that and don't expend time writing and/or testing against anything else and then trying to shoehorn it into the project codebase. Some ideas in a collaborative project just have to wait until the time arises. Plus trying to diagnose issues not related to the set environment.

Also on the CI/CD theme need to consider recompiling libsimulators when changes to radios. As these are currently package with Companion so would need a new download process. This could potentially be added to existing Companion nightly function but as always some thought needed and consideration in the context in the use of EdgeTX Flasher.

pfeerick · 2021-08-24T22:12:05Z

@elecpower Since Companion is your baby ATM, I'll defer to you... I take it there are compatability issues making it so this PR needs more than re-signing with current codebase and merging?

pfeerick · 2021-08-24T22:13:52Z

I also agree with not using ifdefs just to silence warnings - better to find a compiler flag to shut it up where possible ;)

elecpower · 2021-08-25T00:09:36Z

I take it there are compatability issues making it so this PR needs more than re-signing with current codebase and merging?

Yes as per previous comments. Also, I do not get these warnings using the EdgeTX build environment so it would appear they are specific each dev's build environment.
IMHO we 'fix' those that pop up in the official build environment otherwise there will be PR after PR for individual dev issues that potentially cause issues for other dev's environments or even the official build environment.
These are annoying warnings not errors. But if you stray from the official path then be it on your head.
That being said in the next day or so I will recheck Qt version compatibility with OTX and flag any that are incompatible in addition to the one already noted.

pfeerick · 2021-08-25T01:02:10Z

Indeed... we can only support the documented/official dev setup, anything else will only lead to never-ending chaos. Once you've had a chance to review, lets give @sfjuocekr a chance to get this PR up to date and see if we can get it merged.

To that end, I think I'm going to have to say we can't accept the changes to tools/setup_buildenv_ubuntu20.04.sh in this form, as ubuntu21 is not a supported build environment as yet - and those changes wouldn't be consistent with the script anyway, since it is specifically for ubuntu 21.04 ;)

I would suggest capturing the script in it's current form (as it has changed somewhat in the mean time, make your changes, and re-adding it as setup_buildenv_ubuntu21.04_unsupported.sh, whilst adding a caution at the start and end of the run that this is not an officially supported build configuration, so use at your own risk. I would like to keep it rather reject outright, as it will allow for this build environment to actually get tested. :) Although I would suspect the next supported build environment would be 22.04 (next LTS). Your thoughts @raphaelcoeffic ?

elecpower · 2021-08-25T01:35:00Z

@pfeerick @raphaelcoeffic Since EdgeTX is bleeding edge and I agree we want contributions from all quarters, how about an unsupported resources area for scripts and How To instructions. Some really good ideas/techniques/tricks/etc may come from those and make it into the official environment?

elecpower · 2021-08-25T07:31:27Z

 #endif

-    dfuArgs = g.dfuArguments().split(" ", QString::SkipEmptyParts);
+    dfuArgs = g.dfuArguments().split(" ", Qt::SkipEmptyParts);


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T07:31:42Z

 void burnConfigDialog::on_dfuArgs_editingFinished()
 {
-    dfuArgs = ui->dfuArgs->text().split(" ", QString::SkipEmptyParts);
+    dfuArgs = ui->dfuArgs->text().split(" ", Qt::SkipEmptyParts);


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T09:46:10Z

 {
  QPrinter printer;
-  printer.setPageMargins(10.0, 10.0, 10.0, 10.0, printer.Millimeter);
+  printer.setPageMargins({10.0, 10.0, 10.0, 10.0}, QPageLayout::Millimeter);


elecpower · 2021-08-25T09:46:22Z

  QPrinter printer;
  QString filename = QFileDialog::getSaveFileName(this, tr("Select PDF output file"), QString(), "Pdf File(*.pdf)");
-  printer.setPageMargins(10.0, 10.0, 10.0, 10.0, printer.Millimeter);
+  printer.setPageMargins({10.0, 10.0, 10.0, 10.0}, QPageLayout::Millimeter);


elecpower · 2021-08-25T09:46:50Z

+  printer.setPageMargins({10.0, 10.0, 10.0, 10.0}, QPageLayout::Millimeter);
  printer.setOutputFormat(QPrinter::PdfFormat);
-  printer.setOrientation(QPrinter::Landscape);
+  printer.setPageOrientation(QPageLayout::Landscape);


elecpower · 2021-08-25T09:53:15Z

  unsigned int result = 0;
  const Firmware * base = getFirmwareBase();
-  QStringList options = id.mid(base->getId().length()+1).split("-", QString::SkipEmptyParts);
+  QStringList options = id.mid(base->getId().length()+1).split("-", Qt::SkipEmptyParts);


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T09:54:28Z


  QObject::connect(btn, &QToolButton::clicked, [=]() {
-    QString dir = QFileDialog::getExistingDirectory(parent, fsw->property("fileDialogTitle").toString(), le->text(), 0);
+    QString dir = QFileDialog::getExistingDirectory(parent, fsw->property("fileDialogTitle").toString(), le->text(), {});


elecpower · 2021-08-25T09:54:59Z

    }
  }
-  qDebug() << QString().sprintf("%s: OK", getName());
+  qDebug() << QString().asprintf("%s: OK", getName());


elecpower · 2021-08-25T10:08:52Z

  bool added = false;
  // Convert set into list and sort it alphabetically case insensitive
-  QStringList list = QStringList::fromSet(set);
+  QStringList list = QList<QString>(set.begin(), set.end());


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T10:09:23Z

    QString newFilename = logFilename;
    newFilename.append(QString("-Session%1.csv").arg(index));
-    QString filename = QFileDialog::getSaveFileName(this, "Save log", newFilename, "CSV files (.csv);", 0, 0); // getting the filename (full path)
+    QString filename = QFileDialog::getSaveFileName(this, "Save log", newFilename, "CSV files (.csv);", 0, {}); // getting the filename (full path)


elecpower · 2021-08-25T10:19:43Z

 QString MainWindow::seekCodeString(const QByteArray & qba, const QString & label) const
 {
-  int posLabel = qba.indexOf(label);
+  int posLabel = qba.indexOf(label.toUtf8());


elecpower · 2021-08-25T10:26:12Z


  if (!m_options.includeFilter.isEmpty() && m_options.includeFilter != "*")
-    m_dirIteratorFilters = m_options.includeFilter.split(',', QString::SkipEmptyParts);
+    m_dirIteratorFilters = m_options.includeFilter.split(',', Qt::SkipEmptyParts);


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T10:26:25Z


  if (!m_options.excludeFilter.isEmpty()) {
-    for (const QString & f : m_options.excludeFilter.split(',', QString::SkipEmptyParts))
+    for (const QString & f : m_options.excludeFilter.split(',', Qt::SkipEmptyParts))


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T10:28:12Z

 void AutoBitsetCheckBox::updateValue()
 {
  if (m_field)
+  {


Already fixed in OTX and Companion 2.5

elecpower · 2021-08-25T10:28:52Z


    // determine if we have a model number
-    QStringList parts = line.split(QRegExp("\\s+"), QString::SkipEmptyParts);
+    QStringList parts = line.split(QRegExp("\\s+"), Qt::SkipEmptyParts);


Needs Qt 5.14 - change not supported

elecpower · 2021-08-25T10:30:37Z

  QDebugStateSaver saver(d);
  d << "RadioWidget::RadioWidgetState: type=" << o.type << "; index=" << o.index
-    << "; value=" << o.value << "; flags=0x" << hex << o.flags;
+    << "; value=" << o.value << "; flags=0x" << Qt::hex << o.flags;


Support these scope qualifiers

elecpower · 2021-08-25T10:34:38Z

  QSpacerItem * item = new QSpacerItem(1,1, QSizePolicy::Fixed, QSizePolicy::Expanding);

-  ui->curvesLayout->addItem(item,limit + 1, 1, 1, 1, 0);
+  ui->curvesLayout->addItem(item, limit + 1, 1, 1, 1);


elecpower · 2021-08-25T10:36:30Z

@pfeerick finished my review of the Companion code. Where no comment then previous comments applies for same change

pfeerick · 2021-08-29T01:49:05Z

@sfjuocekr Did you want to work on this further. Otherwise, I can add a commit with the reviewed changes in the #555 PR so that it is included as part of that (if @elecpower is ok with that). Your build script would go in https://github.com/EdgeTX/playground, a new repo to host more experimental/unsupported stuff, and will be the first addition.

I'll give say three days for a reply before proceeding further...

elecpower · 2021-08-29T02:29:47Z

@pfeerick @sfjuocekr this PR will need to be rebased on #555 and conflicts resolved.
I have a couple more commits for 555 to push after a bit more testing.

sfjuocekr · 2021-09-01T12:17:24Z

I've had to deal with some unfortunate events in my personal life, sorry for not responding.

The resolve conflicts button is grayed out for me, I'll try to go over the changes and touch up the copy paste errors. No guarantees tho, I've just lost one of my best friends :(

pfeerick · 2021-09-01T12:29:08Z

My condolences. 😢 Take your time...

Yeah, the changes to setup_buildenv_ubuntu20.04.sh were probably too substantial for it to want to let you resolve online. You'll be able to resolve that by removing it - that can get merged into the playground repo instead. PR for exactly that. Then you can focus just on the companion rebase/changes.

Add builder from EdgeTX/edgetx#446

raphaelcoeffic · 2022-01-09T15:46:00Z

By the way, adding real support for Mac M1 seems to force us to update to C++17 and Qt 6.2 (only supported version so far).

elecpower · 2022-01-09T20:22:05Z

By the way, adding real support for Mac M1 seems to force us to update to C++17 and Qt 6.2 (only supported version so far).

That is a whole lot of pain as significant changes from Qt 5 to 6.

raphaelcoeffic · 2022-01-09T20:34:42Z

By the way, adding real support for Mac M1 seems to force us to update to C++17 and Qt 6.2 (only supported version so far).

That is a whole lot of pain as significant changes from Qt 5 to 6.

I know… but it’s not like we have a choice…

pfeerick · 2022-01-10T00:30:20Z

Well, we, do... who cares about the Mac M1 :-P That's just one variant of one OS supported... I'm not saying this shouldn't happen, but I don't see it even being worth started on until 2.8/9 ... especially given that means dropping support for Windows 7/ 8 & 10 (pre 1809), macOS 10.13 , and linux distros from 18 -> 20 releases.

raphaelcoeffic · 2022-01-10T05:40:49Z

Well, we, do... who cares about the Mac M1 :-P That's just one variant of one OS supported... I'm not saying this shouldn't happen, but I don't see it even being worth started on until 2.8/9 ... especially given that means dropping support for Windows 7/ 8 & 10 (pre 1809), macOS 10.13 , and linux distros from 18 -> 20 releases.

Whoa! Really? This is crazy! Qt said they might support Apple silicon in 5.15, but it seems to be only for the “commercial” version, whatever that means.

elecpower · 2023-03-10T20:57:41Z

Upgrade to Qt 5.15.2 covered by #3339 and some of the radio changes have been superseded by PRs moving to new ui, etc

Fixed a bunch of compiler warnings and deprecated functions.

c61b553

pfeerick added the companion Related to the companion software label Jul 14, 2021

raphaelcoeffic changed the base branch from 2.4 to main July 20, 2021 11:34

raphaelcoeffic and others added 3 commits July 22, 2021 09:01

[color] fix statistics view (now displays proper numbers)

98b1cd3

[color] Fix timer display problem

e4b6209

Fix colorlcd FRM302 (#8624)

09a6580

raphaelcoeffic requested a review from elecpower July 31, 2021 06:00

elecpower reviewed Aug 1, 2021

View reviewed changes

elecpower reviewed Aug 25, 2021

View reviewed changes

pfeerick added the question Further information is requested label Aug 29, 2021

pfeerick added a commit to EdgeTX/playground that referenced this pull request Sep 1, 2021

Create setup_buildenv_ubuntu21.04.sh

6ef7c38

Add builder from EdgeTX/edgetx#446

pfeerick mentioned this pull request Sep 1, 2021

Create setup_buildenv_ubuntu21.04.sh EdgeTX/playground#1

Open

elecpower mentioned this pull request Sep 11, 2021

Windows companion build #706

Closed

pfeerick self-assigned this Oct 7, 2021

Merge branch 'EdgeTX:2.4' into 2.4

815a632

pfeerick removed their assignment Mar 18, 2022

pfeerick removed the question Further information is requested label Mar 17, 2023

pfeerick closed this Mar 17, 2023

Uh oh!

Uh oh!

Conversation

sfjuocekr commented Jul 14, 2021

Uh oh!

raphaelcoeffic commented Jul 14, 2021

Uh oh!

sfjuocekr commented Jul 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elecpower commented Aug 1, 2021

Uh oh!

pfeerick commented Aug 1, 2021

Uh oh!

raphaelcoeffic commented Aug 1, 2021

Uh oh!

sfjuocekr commented Aug 1, 2021

Uh oh!

elecpower commented Aug 1, 2021

Uh oh!

elecpower commented Aug 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfeerick commented Aug 24, 2021

Uh oh!

pfeerick commented Aug 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elecpower commented Aug 25, 2021

Uh oh!

pfeerick commented Aug 25, 2021

Uh oh!

elecpower commented Aug 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elecpower commented Aug 25, 2021

elecpower commented Aug 1, 2021 •

edited

Loading

pfeerick commented Aug 24, 2021 •

edited

Loading

pfeerick commented Aug 29, 2021 •

edited

Loading

pfeerick commented Sep 1, 2021 •

edited

Loading