split vizModel to a separate file #5094
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
|
Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!
|
viambot
added
safe to test
|
When this is merged, we'll change penguinland#1 to be a PR into the main fork, and it'll be easier to review than jumbling it all together in here. |
|
Github_actions_bot is overzealous: although I touched the file that defines |
AGanguly13
approved these changes
Jun 30, 2025
penguinland
added a commit
that referenced
this pull request
Jul 1, 2025
#5096) Adwait and I pair programmed this; thanks for his help! The main changes are: - The `vizModel` struct no longer stores a `robot.Robot` within it. Instead, it stores a `logging.Logger` and a function that takes camera names and returns camera objects. - The old `NewService()` function has been renamed `DeprecatedNewService()`. Like before, it takes a `robot.Robot` as an argument, and can be used with `DeprecatedRobotConstructor` when registering a service. - There is a new function now named `NewService()`. It takes a `resource.Dependencies` as an object, and can be used with `Constructor` when registering a service. - The file `deprecated_vision_service_builder.go` (created in #5094) has been renamed to non-deprecated `vision_service_builder.go` This compiles and the tests pass, and the mlmodel vision service still works. However, nothing (yet) uses the new, non-deprecated `NewService()`. That can happen in future PRs. We have created a new function with a new type signature using an old name. However, we don't need to worry about someone outside of RDK using this function in their module and having the module break when they upgrade RDK versions: the reason RSDK-11093 was filed in the first place was that the old code was not usable in modules at all, because `DeprecatedNewService()` doesn't play well with modules!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
vision.go was super long, and half of it was unrelated to the other half. So, it's now 2 files. This will make it easier to think about fixing RDSK-11093, which is about updating
vision.NewService()to not use the deprecated constructor.No changes to functionality are intended: I just took a file and split it in half.