-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ped plugin #620
Fix ped plugin #620
Conversation
modified: src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp modified: src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
modified: src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
@@ -633,7 +633,7 @@ int CommandPlugin::WSCallbackBASE64( | |||
argsList[arg.first] = arg.second.data(); | |||
} | |||
} | |||
catch (exception argsEx) | |||
catch (exception &argsEx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catch(const std::exception &argEx)
Also where you just address a code smell or why are there changes to this file at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just to address a code smell
modified: src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp modified: src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
Quality Gate failedFailed conditions |
modified: examples/tmx-exampleapps/EmptyPlugin/src/EmptyPlugin.cpp modified: ext/build.sh modified: src/tmx/TmxApi/CMakeLists.txt modified: src/tmx/TmxApi/tmx/messages/message_document.hpp deleted: src/tmx/TmxApi/tmx/pugixml/pugiconfig.hpp deleted: src/tmx/TmxApi/tmx/pugixml/pugixml.cpp deleted: src/tmx/TmxApi/tmx/pugixml/pugixml.hpp deleted: src/tmx/TmxApi/tmx/pugixml/readme.txt
modified: src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp modified: src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
ext/build.sh
Outdated
@@ -72,3 +72,11 @@ make -j${numCPU} | |||
make install | |||
popd | |||
|
|||
# pugixml | |||
pushd /tmp | |||
git clone https://github.com/zeux/pugixml.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to target a specific release to avoid new releases breaking our build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, changed to v1.14
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left several small comments. For ones labeled NIT please address or don't address these as you see fit. For all others please address them.
modified: src/v2i-hub/PedestrianPlugin/manifest.json modified: src/v2i-hub/PedestrianPlugin/scripts/sendPSM.py modified: src/v2i-hub/PedestrianPlugin/src/PedestrianPlugin.cpp modified: src/v2i-hub/PedestrianPlugin/src/include/PedestrianPlugin.hpp
Quality Gate failedFailed conditions |
PR Details
Description
These changes address a memory leak in the WebService/PSM functionality of the plugin. It also allows a user to switch between using FLIR(WebSocket) and PSM(WebService) without crashing or generating segmentation faults.
Related Issue
#497
Motivation and Context
Users of this plugin have noticed memory loss and error code generation while using this plugin. Users have also requested a fix for the #497 issue.
How Has This Been Tested?
Tested using a script to send 1000 PSM XMLs to the plugin. No messages were lost.
Tested using local FLIR camera to ensure functionality was maintained.
Switched between DataProviders (PSM/FLIR) in various combinations to ensure threads were closed or joined properly.
Types of changes
Checklist:
V2XHUB Contributing Guide