COVESA / dlt-viewer

Diagnostic Log and Trace viewing program
Other
425 stars 240 forks source link

Fix more compilation warnings #499

Closed vifactor closed 1 month ago

vifactor commented 2 months ago

Mainly due to deprecated methods and integer comparison of different signedness.

vifactor commented 2 months ago

I used one c++17 feature but then noticed that project is set to built with c++11. Is this a strong requirement?

Edit: it seems important, windows CI fails. Fixed

alexmucde commented 2 months ago

@vifactor Thanks for your contribution. Is it possible to add a signed off statement with your name and eMail address and in your last or a new commit message?

vifactor commented 2 months ago

@alexmucde sure, done

alexmucde commented 1 month ago

@vifactor Sorry i was working on a parallel task. Can you please rebase to master? The file src/dltimporter.cpp is now moved to qdlt/qdltimporter.cpp

vifactor commented 1 month ago

@alexmucde rebased

vifactor commented 1 month ago

@alexmucde do I need to do anything else to make this merged?

alexmucde commented 1 month ago

@vifactor Should be fine now, but i would like to test it before integration. Have you already tested the affected parts of the SW, e.g. MF4 import?

vifactor commented 1 month ago

@alexmucde I thought my changes were relatively trivial. But, of course, you are right better to verify even trivial changes. Unfortunately, I do not think I have MF4 files. Do you have some samples?

vifactor commented 1 month ago

@alexmucde I found some mf4 files here: https://canlogger.csselectronics.com/canedge-getting-started/ce2/_downloads/de00db1faa001b51fce93666ebaba20e/mf4-sample-data-v2.1.zip

I tried dlt-viewer v2.21.2 and the version from master. In both cases when I import files, no logs are seen in the viewer UI, however console logs show that something gets imported. Maybe I simply do not know how to check that functionality. Would appreciate some help

vifactor commented 1 month ago

@alexmucde would this PR have higher chances to be merged sooner if I move so far untested changes for qdlt/qdltimporter.cpp into a separate PR?

alexmucde commented 1 month ago

@vifactor I have tested MF4 import and it is still working. I will merge, but further testing needed.