Artifacts in the opened v2 dlt file
svlad-90 opened this issue · 6 comments
I have a DLT Message Analyzer plugin feature that converts the arbitrary textual file separated with new lines into the dlt format. I support both the v1 and v2 dlt protocol versions.
Using the latest dlt-viewer, the files manually created with my plugin ( of v2 version ) are opened with artifacts.
The issue looks as the following:
Commit, after which I have this side effect:
566b4ff
I've analyzed the commit and found that you've added the 'supportDLTv2' parameter to the 'QDltMsg::setMsg' method. Some code was refactored to provide this parameter from the settings.
But this code left with unconditional usage of the 'false' default value:
=> Is that OK? Wouldn't that lead to the unconditional interpretation of the v2 file as v1 in case you work with a pre-saved file that is opened without connection to the target board? Adaptation of only the QDltConnection is not enough. QDltFile should also be adapted to this change.
I've changed the above-mentioned two lines to unconditional true for the 'supportDLTv2' parameter:
msg.setMsg(buf, true, true);
and
result = msg.setMsg(data, true, true);
After that, my v2 file ( and also v1 files ) can be opened again without the artifacts:
I'm sure that there are better approaches than 'unconditional true.' I'm not proposing the fix; I'm only highlighting a bug in the recent implementation.
=> Please also remember that both dlt message and dlt file are the types that are available for the plugin developers. It is part of the public API. If you've exposed a new 'supportDLTv2' parameter on that level, it would be better to provide a way to get the corresponding settings property.
=> Please try to fix it as part of the v2.26.0 release. It is a significant bug that might impact many users already working with the v2 dlt files.
I'll stick strictly to version 2.25.0 for my current project. Usage of the unstable HEAD revision was not the best idea. ))
@svlad-90 Sorry that my change causes some confusion. We had some issue with some devices, that's why i disabled the dltv2 support by default. Now you have to enable it in the settings.
I did not know that anyone out there is alreade using dltv2.
We must find a solution which fits all needs for everybody. When calling setMsg the settings must be considered now. It could be, that i have forgot some parts of the code. We should fix this until the next stable version.
Hi @alexmucde,
We had some issue with some devices, that's why i disabled the dltv2 support by default. Now you have to enable it in the settings.
It is not an issue that I should now select dltv2 support in the settings. I was smart enough to check your commits and figure that out.
The issue is that the artifacts are on the screen when I open the v2 *.dlt file, even when I turn on dltv2 support.
To be clear, there are the following scenarios available:
- If the dltv2 support is turned on:
1.1 the dltv1 files are successfully parsed
1.2 the dltv1 data is successfully received from the connection to the target board
1.3 the dltv2 files are successfully parsed
1.4 the dltv2 data is successfully received from the connection to the target board - If the dltv2 support is turned off:
2.1 the dltv1 files are successfully parsed
2.2 the dltv1 data is successfully received from the connection to the target board
2.3 the processing of the dltv2 data may fail or be shown with artifacts, as it is parsed with the v1 parsing algorithm
With the current implementation, the use-case 1.3 doesn't work, which is unexpected.
I did not know that anyone out there was already using dltv2.
I implemented its support in my plugin when dlt-viewer introduced the support of dltv2. In my case, the plugin creates *.dlt files from any textual data. In other words, data is not retrieved from the dlt-daemon. I'm analyzing external logs ( mainly Android logcat ) received from a Xen hypervisor domain with no installed dlt-daemon. I'm generating v2 by default to be on the same page with the latest AUTOSAR specifications. ))
We must find a solution which fits all needs for everybody. When calling setMsg the settings must be considered now. It could be, that i have forgot some parts of the code. We should fix this until the next stable version.
Great! I'm looking forward to getting the fix.
But please feel free to take your time. My plugin can generate v1 *.dlt files as well, which works fine when the dltv2 support is turned off in the dlt-viewer. So I can move on with my daily tasks. Nothing critical.
Hi @alexmucde,
Thanks for your feedback! I appreciate your support, Alex! ))
I've checked the fix. It is functional. The point 1.3 from my previous message now works fine.
Generally, it would be good to have read-only access to the settings->supportDLTv2Decoding property on the plugin level. I have such calls where I'm forming the QDltMsg on my own within the plugin:
It would be great to consider the 'supportDLTv2Decoding' dlt-viewer setting on the plugin side to have a more straightforward flow. Currently, plugins have no access to dlt-viewer settings.
An additional feature can be implemented to provide plugins with read-only access to all dlt-viewer settings. But that can be done later on, outside of the scope of this issue. Someone ( me ) may someday develop it and contribute such a change to the dlt-viewer.
=> Still, if possible, could you extend your change with an additional 'bool QDltFile::getDLTv2Support() const;' method?
That will allow me to get access to the needed information as each applied file is passed to the plugin with the following call:
https://github.com/COVESA/dlt-viewer/blob/master/qdlt/plugininterface.h#L187
I have no other findings or proposals regarding the target PR.
Done, added getter function.
Hi @alexmucde, thanks for considering my suggestion. This change can be merged, I guess.