feat: new api for creating and connecting datasets/robots#594
feat: new api for creating and connecting datasets/robots#594sdas-neuraco wants to merge 6 commits into
Conversation
37ccb61 to
94d5354
Compare
|
PR title and commit messages must follow conventional commit format: : . Valid prefixes: feat, fix, chore, docs, ci, test, refactor, style, perf. |
This reverts commit 3686603. fix: WIP fix: WIP
Hi I am confused why the recording control is moved into dataset. Because dataset itself cannot really do any recording but the robot can control the start&end of recording. For example if we do recording we have It will also feel weird when doing inference, robot is collecting the input data and not the dataset. |
| The initialized and connected robot instance. | ||
| The initialized robot instance. | ||
|
|
||
| Raises: |
There was a problem hiding this comment.
Only raise error if exist_ok=False
| warn( | ||
| f"This robot '{robot.name}' is archived. Was this intentional?", | ||
|
|
||
| try: |
There was a problem hiding this comment.
here robot name and robot_id are treated separately but the logic is the same, we can combine them onto one try-except-else logic
|
@Ke-Wang1017 This was based on the votes in the discussion: https://neuracoreai.slack.com/archives/C09D6DK95L5/p1777483613625719 The dataset does the recording because you record and save to a dataset not to a robot. The logging is to the robot (for live viewing), hence its keeping to per robot. |
ypang-neuraco
left a comment
There was a problem hiding this comment.
I think with this change the way we handle datasets becomes inconsistent with how we handle robots. They are similar where we have an active dataset and a active robot that can be connected to. With the robot, when logging data we always log to the active robot without needing to have a robot instance. But with dataset, we now need to have a dataset instance to call start/stop recording.
If the expectation is that only one dataset should be active, connect_dataset should not return the dataset instance and instead should just set the active dataset. That way the user cannot start recording on a non-active dataset, which is not allowed anyway.
Features
nc.create_robot(...)andnc.connect_robot(...)calls so creation no longer implicitly sets the active robot.nc.connect_dataset(...)and moves recording controls onto connected dataset objects:dataset.start_recording(...),dataset.stop_recording(...), anddataset.cancel_recording(...).exist_oksupport tonc.create_robot(...)andnc.create_dataset(...)so callers can safely reuse existing resources when requested.Items
I will squash after approval