-
Notifications
You must be signed in to change notification settings - Fork 16
Migrate bitbots_vision to use generate_parameter_library with optimized parameter passing #722
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
base: main
Are you sure you want to change the base?
Conversation
generate_parameter_library
instead of the custom build params.py. Make all nessesary changes to the source code to use the new config datasetructure. Add it as a ros dependency and adapt the build scripts. Ignore...generate_parameter_library
Co-authored-by: Flova <[email protected]>
…er_library integration Co-authored-by: Flova <[email protected]>
generate_parameter_library
Co-authored-by: Flova <[email protected]>
Co-authored-by: Flova <[email protected]>
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.
@copilot Please address the feedback.
If you want to test it with ros etc. look at the devcontainer and make a ros docker as described there
return True | ||
|
||
# Compare YOEO-related parameters using direct attribute access | ||
return ( |
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.
Add a parameter hierachy. If you do this you can have a substruct with all yoeo parameters and compare this directly instead of listing all params in an error prone way
Also remove the caching logic.
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.
Implemented in commit 0a56fc6. Added parameter hierarchy with yoeo:
sub-structure in YAML. Updated parameter comparison to simply compare cls._config.yoeo != new_config.yoeo
instead of listing individual parameters. The caching logic was already removed in the restructuring.
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.
@copilot Could you just pass the substruct to the subcomponent?
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.
Implemented in commit 538a401. Now passing the yoeo substruct directly to YOEO components:
- Changed
YOEOObjectManager.configure(config)
→YOEOObjectManager.configure(config.yoeo)
- Updated all YOEO handlers to accept
yoeo_config
parameter directly - Direct parameter access:
yoeo_config.framework
,yoeo_config.conf_threshold
, etc. - Moved
caching
parameter underyoeo:
hierarchy for complete encapsulation
YOEO components now only receive their relevant parameter substruct instead of the full config object.
bitbots_vision/bitbots_vision/vision_modules/yoeo/yoeo_handlers.py
Outdated
Show resolved
Hide resolved
bitbots_vision/bitbots_vision/vision_modules/yoeo/yoeo_handlers.py
Outdated
Show resolved
Hide resolved
bitbots_vision/bitbots_vision/vision_modules/yoeo/yoeo_handlers.py
Outdated
Show resolved
Hide resolved
@copilot If you want to test it with ros etc. look at the devcontainer and make a ros docker as described there |
…d rework ros_utils Co-authored-by: Flova <[email protected]>
Co-authored-by: Flova <[email protected]>
This PR migrates the
bitbots_vision
package from its custom parameter system to use the standardizedgenerate_parameter_library
, bringing it in line with other packages likebitbots_path_planning
.Problem
The
bitbots_vision
package was using a customParameterGenerator
class inparams.py
that manually handled parameter declaration and type conversion. This approach had several limitations:generate_parameter_library
Solution
Replaced the custom parameter system with
generate_parameter_library
:1. Parameter Schema Definition with Hierarchy
Created
config/vision_parameters.yaml
defining 20 parameters with proper typing, validation, and hierarchical organization:yoeo:
structure):yoeo.model_path
,yoeo.nms_threshold
,yoeo.conf_threshold
,yoeo.framework
,yoeo.caching
one_of<>: [["pytorch", "openvino", "onnx", "tvm"]]
2. Build System Integration
generate_parameter_library
dependency topackage.xml
setup.py
to callgenerate_parameter_module()
at build time.gitignore
entry for generated parameter files (vision_parameters.py
)3. Code Architecture Updates with Type Safety
YOEOVision
class (inherits fromNode
)config.yoeo.framework
)4. Complete Subsystem Migration with Optimized Parameter Passing
yoeo
substruct (config.yoeo
) instead of full config for better encapsulationyoeo_config
parameter with direct access (yoeo_config.framework
,yoeo_config.caching
)cls._config != new_yoeo_config
)create_or_update_subscriber_with_config()
that works directly with config objects5. Code Quality Improvements
6. Legacy Code Removal
ParameterGenerator
class andparams.py
fileBenefits
yoeo.*
sub-parameters including caching for complete encapsulationTesting
The migration provides complete modernization of the parameter system with full parameter hierarchy, type safety, validation support, and optimized parameter passing throughout all subsystems, following the best practices established by the generate_parameter_library framework.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.