-
Notifications
You must be signed in to change notification settings - Fork 3
Tensorflow TLR GUI #19
base: as/111_live_traffic
Are you sure you want to change the base?
Conversation
* Add plugin edit tool and update plugins * Update the code for compatibility with python3 * Skip test of unbuilt packages * Two other minor rebase fixes. * First pass at adding region_tlr_tensorflow and roi_pub. * Adding 1.13 to FindTensorFlow.cmake. * Removing FindCuDNN.cmake. Changing FindTensorFlow variables. * Adding proper Python folder structure. * Adding new nodes to interface.yaml. * Minor formatting fix. * Making encoding in roi_pub type-specific. * Fixing roi_pub for images with rggb8 format. * Launch file for region_tlr_tensorflow * Renaming launch file for consistency. * Removing unused portion of launch file.
* First run at replacing region_tlr_tensorflow with new version. * Fixing order-of-operations CMake bug in TLR. * Fixing another order-of-operations issue in TLR. * Adding main function and fixing srv install. * Fixing launch file for new region_tlr_tensorflow. * Making tensorflow_tlr.py a class. * python global variable fixes in tlr * python self instance ref fixes
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.
Adding the launch file to the GUI is fine but the configurable params probably aren't worth it for now.
dash : '' | ||
delim : ':=' | ||
|
||
|
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 currently won't work for 2 reasons:
- This node no longer listens to the
/config/superimpose
topic. All it did was pop up an OpenCV window so we removed it. - These
/config
topics are theoretically for things that can be dynamically reconfigured. The NN model is not one of those things right now. We didn't design the node to handle changing the model on-the-fly - just at start-up. Same goes for the camera_id at the moment - though that would be easier to change.
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.
The NN model path is a static option and it is not going to be part of the /config/superimpose topic (I can get rid of that). We can leave the model path on there to provide the option to set a path to the model from the GUI. We can include other launch file parameters in here once we have the final version, even if not to dynamically reconfigure right now. Not all nodes in autoware have a config message yet. What do you think @JWhitleyAStuff ?
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.
That doesn't make sense. We have seen that changes to these GUI fields publish a configuration message, which we don't support. There is no point in adding these here since they only change that configuration message. Is there documentation on what these fields mean? I was to understand that these fields were only for dynamic reconfiguration. Do you have some information that says different?
0cc154c
to
9ab8043
Compare
Status
PRODUCTION / DEVELOPMENT
Description
Adding Tensorflow TLR to RT GUI.
Todos