Skip to content

Conversation

njooma
Copy link
Member

@njooma njooma commented Jun 6, 2023

Modular resources could only depend on components. NOW they can depend on services too :)

@njooma njooma requested a review from a team as a code owner June 6, 2023 15:47
Copy link
Contributor

@maximpertsov maximpertsov left a comment

Choose a reason for hiding this comment

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

We should add a test that includes a service in tests/test_module.py to validate this behavior - otherwise looks good!

Comment on lines 99 to 100
component = await self._get_resource(rn)
deps[rn] = component
Copy link
Contributor

Choose a reason for hiding this comment

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

was originally gonna say that the component variable name is wrong now, but maybe this can just be one line:

Suggested change
component = await self._get_resource(rn)
deps[rn] = component
deps[rn] = await self._get_resource(rn)

@njooma njooma requested a review from maximpertsov June 6, 2023 19:02
@njooma
Copy link
Member Author

njooma commented Jun 6, 2023

Also fixed an issue where adding a movement sensor/getting a status for a movement sensor would sporadically fail based on the ordering of whether sensor or movement_sensor came first (if sensor came first, then it would get added and movement_sensor would fail. But if movement_sensor got added first, then sensor would automatically get added as part of the movement_sensor).

Doing this required reorganizing some imports to avoid circular imports

@njooma njooma merged commit cd810d6 into viamrobotics:main Jun 6, 2023
@njooma njooma deleted the modular-resource-dependencies branch June 6, 2023 20:48
@clintpurser
Copy link
Member

belated shipit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants