CARMACloudPlugin: Fix the issue to unzip the compressed TCMs #602
Conversation
| std::list<std::string> tcm_sl = {}; | ||
| if (isCompressed) | ||
| { | ||
| tcm_sl = FilterTCMs(tcm); |
There was a problem hiding this comment.
Is there a reason we only call FilterTCMs if the message was originally compressed? If so, it's probably worth at least a comment.
There was a problem hiding this comment.
The compressed file has list of TCMs wrapped inside the tags. All these are in a big XML payload. This filter tcms is to remove the list tags and only return a list a TCMs objects. There are comments in the header file:
There was a problem hiding this comment.
So if not compressed it is guranteed to be a list of a single TCM but if it is compressed it is multiple TCMs in a single XML that have to be parsed out individually? Is that correct?
There was a problem hiding this comment.
Yes about compressed. For not compressed, it is one TCM at a time.
There was a problem hiding this comment.
So then what is this configuration parameter for
| strm.zalloc = nullptr;//Refer to zlib docs (https://zlib.net/zlib_how.html) | ||
| strm.zfree = nullptr; | ||
| strm.opaque = nullptr; | ||
| strm.zalloc = Z_NULL;//Refer to zlib docs (https://zlib.net/zlib_how.html) |
There was a problem hiding this comment.
Whitespace in this function looks a little weird, but could just be github's rendering.
| return; | ||
| } | ||
| PLOG(logINFO) << "Received TCM bytes size: " << st.size()<< std::endl; | ||
|
|
||
| string tcm = ""; |
There was a problem hiding this comment.
Just for consistency's sake, is there a reason we use string here and std::string below?
| } while (Z_STREAM_END != isDone); //Reach the end of stream to be uncompressed | ||
| }else{ | ||
| isDone = inflate(&strm, Z_NO_FLUSH); | ||
| outBuf.append(buffer, BUFFER_SIZE - strm.avail_out); |
There was a problem hiding this comment.
Nit: Indentation looks off.
| @@ -376,7 +377,7 @@ void CARMACloudPlugin::TCMAckCheckAndRebroadcastTCM() | |||
| } | |||
| else | |||
| { | |||
| PLOG(logDEBUG) << "NO TCMs to broadcast." << std::endl; | |||
| PLOG(logDEBUG4) << "NO TCMs to broadcast." << std::endl; | |||
There was a problem hiding this comment.
Would it be useful to have a status that evaluates to the number of TCMs currently broadcasting rather than this log statement.
There was a problem hiding this comment.
Yeah, that might be a nice feature to add. This is an indicator that no more TCMs that is left to broadcast.
| isDone = inflate(&strm, Z_NO_FLUSH); | ||
| outBuf.append(buffer, BUFFER_SIZE - strm.avail_out); | ||
| } while (Z_STREAM_END != isDone); // Reach the end of stream to be uncompressed | ||
| }else{ | ||
| PLOG(logWARNING) << "Error initalize stream. Err code = " << err << std::endl; |
There was a problem hiding this comment.
Do we still want to execute the last line of this method if this error happens?
There was a problem hiding this comment.
It seems useful to know that the stream is not readable and what the actual error is. What do you suggest?
There was a problem hiding this comment.
I am asking whether we want the statements on L634 and L635 to execute in this error case. I have no problem with the logging except if it is an error we may want to make it error level and not warning? Just curious whether we want to stop doing everything in this case or not.
There was a problem hiding this comment.
Updated it to error type and handled it silently by ignoring it for now.
|
PR Details
Description
Fix issue with CARMACloudPlugin unzip the compressed TCM payload. The unzip function did not append the correct numbers of characters. Add 32 to WinBits to allow header detection. Remove toLocal8Bit() conversion as the xml is by default sent as utf-8 encoding.
Related Issue
#601
Motivation and Context
How Has This Been Tested?
Local integration test
Types of changes
Checklist:
V2XHUB Contributing Guide