Skip to content

RSDK-11067: remove resource on alwaysrebuild #458

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.

Sign up for GitHub

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

Merged
lia-viam merged 5 commits into from
Jul 1, 2025
Merged

RSDK-11067: remove resource on alwaysrebuild #458

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5f4dcdc
remove resource on alwaysrebuild
lia-viam Jun 27, 2025
9185837
use resource name
lia-viam Jun 30, 2025
769e664
fix call to reg lookup
lia-viam Jul 1, 2025
2332592
remove rsdk comment
lia-viam Jul 1, 2025
b5578ce
error on model reg not found
lia-viam Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/viam/sdk/module/service.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service {
stoppable->stop();
}

manager->remove(cfg.resource_name());
Copy link
Member

@acmorrow acmorrow Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below, this calls manager->replace_one. The documentation for replace_one indicates that it will only work right if resource is present, and will fail otherwise. So, by removing the resource here, I think it prevents replace_one below from working.

Copy link
Collaborator Author

@lia-viam lia-viam Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh good catch...i will look into what's happening in replace_one because i think from @hexbabe earlier testing the destructor on the original instance was not getting called during this always-rebuild control flow


const std::shared_ptr<const ModelRegistration> reg =
Registry::get().lookup_model(cfg.name());

Expand Down