svlad-90 / DLT-Message-Analyzer

"Extended search" plugin for the DLT-viewer
GNU General Public License v3.0
77 stars 13 forks source link

[DEPENDENCY_UPDATE] Compatibility with Qt6 #166

Closed sevketcaba closed 1 year ago

sevketcaba commented 1 year ago

Is your feature request related to a problem? Please describe. Although dlt-viewer itself can be built with Qt6, DMA does not support it yet

Describe the solution you'd like A relatively simple code refactoring should be more than enough to migrate to Qt6 while still conserving Qt5 support

Describe alternatives you've considered No alternatives available

Additional context No additional context

svlad-90 commented 1 year ago

Hi @sevketcaba, I've just noticed your contributions in my email box.

First of all, thanks a lot for your efforts in creating this issue and a corresponding PR. I do really appreciate it!

I knew that the day will come when compatibility with QT6 will become needed. But I've missed that dlt-viewer already supports QT6 for around half a year.

Also, I never thought that someone would prepare the needed changes instead of me. What a great thing - open source. ))

Could you, please, provide me with information regarding:

Some of the questions are silly. But there were no major updates of the DMA for a long time. So I've got a little bit out of context of the dlt-viewer status. Also, I didn't get a chance to work with QT6. So, do not be surprised. Still, I'm here, totally ready to support you with the change. ))

P.S. I will spend further 1-2 days getting into the context. If I will have further questions - I'll add them as separate comments in this issue.

P.P.S. If we will merge your PR, I'll definitely do additional changes to have a CI/CD session for building the QT6 version, so that we have at least some level of quality assurance.

svlad-90 commented 1 year ago

Hi @sevketcaba,

I've answered some of my own questions:

Starting from which version dlt-viewer supports QT6?

Starting from v2.24.0, which is not released yet. Still, it is almost released. So I can stick CI/CD to the following commit and that should work: https://github.com/COVESA/dlt-viewer/commit/ebb3c355bc0edf2b6c3be84452b8afa4d1a82641

Is your fix backward compatible with the QT5 version?

It should be. But I need to update my CI/CD to check that. Currently, it is based on too old version of dlt-viewer. I've created a separate issue to update the used version of the dlt-viewer: https://github.com/svlad-90/DLT-Message-Analyzer/issues/168

How can I build a QT6 version of the dlt-viewer and DMA? Can I do it on Ubuntu 20.04 LTS? Or should I change my OS to a newer one?

I think I'll create CI/CD for QT6 even before your PR is merged. Tomorrow. That will also allow me to test which OS should be used to actually try it out later.

_Is QTPREFIX some variable on dlt-viewer level, which selects the QT version?

Yes, it is. This flag is selected automatically on the dlt-viewer's level, depending on the available version of QT.

P.S. I will spend a further 1-2 days getting into the context. If I will have further questions - I'll add them as separate comments in this issue.

It took me less to get into the context. ))

svlad-90 commented 1 year ago

Hi @sevketcaba, could you, please, update your qt6 branch so that it contains the following PR? https://github.com/svlad-90/DLT-Message-Analyzer/pull/169

That should unblock the QT5-based CI/CD, which is currently failing. After QT5 pipelines will succeed, I'll develop QT6 ones.

sevketcaba commented 1 year ago

Hi @svlad-90

I'm happy to see that you're still maintaining this repository, and already got the answers for your questions. I've re-based my qt6 branch with the latest commits of your master branch. I hope that helps.

Beside that, you may want to have a look at this repository https://github.com/sevketcaba/dlt-viewer-docker . This may give you some insights to use docker to build the dlt-viewer and DMA with different operating systems and compilers.

Feel free to reach me out if you need any help.

svlad-90 commented 1 year ago

Hi @sevketcaba, I've tried your solution https://github.com/sevketcaba/dlt-viewer-docker. That worked for me to try out QT6 version of dlt-viewer + DMA.

Now I'm convinced, that it works. I've merged your PR. I'll work on the QT6 CI/CD pipeline separately.

If you don't mind, I'll add a link to your repository - https://github.com/sevketcaba/dlt-viewer-docker - in the "installation" section of DMA. It should simplify installation, at least for Linux users.

P.S. Thanks you once again for your contribution.

sevketcaba commented 1 year ago

Hi @svlad-90 glad to hear that. Sure, you can add a link to the repository. I'll try to keep it alive and up-to-date