-
Notifications
You must be signed in to change notification settings - Fork 18
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_configparameter directly - Direct parameter access:
yoeo_config.framework,yoeo_config.conf_threshold, etc. - Moved
cachingparameter 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_visionpackage from its custom parameter system to use the standardizedgenerate_parameter_library, bringing it in line with other packages likebitbots_path_planning.Problem
The
bitbots_visionpackage was using a customParameterGeneratorclass inparams.pythat manually handled parameter declaration and type conversion. This approach had several limitations:generate_parameter_librarySolution
Replaced the custom parameter system with
generate_parameter_library:1. Parameter Schema Definition with Hierarchy
Created
config/vision_parameters.yamldefining 20 parameters with proper typing, validation, and hierarchical organization:yoeo:structure):yoeo.model_path,yoeo.nms_threshold,yoeo.conf_threshold,yoeo.framework,yoeo.cachingone_of<>: [["pytorch", "openvino", "onnx", "tvm"]]2. Build System Integration
generate_parameter_librarydependency topackage.xmlsetup.pyto callgenerate_parameter_module()at build time.gitignoreentry for generated parameter files (vision_parameters.py)3. Code Architecture Updates with Type Safety
YOEOVisionclass (inherits fromNode)config.yoeo.framework)4. Complete Subsystem Migration with Optimized Parameter Passing
yoeosubstruct (config.yoeo) instead of full config for better encapsulationyoeo_configparameter 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
ParameterGeneratorclass andparams.pyfileBenefits
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.