Skip to content

Merge feature/topic-normalize into develop#18

Open
lewkoo wants to merge 17 commits into
developfrom
feature/topic-normalize
Open

Merge feature/topic-normalize into develop#18
lewkoo wants to merge 17 commits into
developfrom
feature/topic-normalize

Conversation

@lewkoo
Copy link
Copy Markdown
Member

@lewkoo lewkoo commented Apr 15, 2026

Summary

Create the PR for the existing feature/topic-normalize branch against develop.

Validation

Branch diff inspected locally.

Copilot AI review requested due to automatic review settings April 15, 2026 07:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the runtime configuration for the Perception stack to use normalized topic namespaces (via HARU_TOPIC_PREFIX) and introduces new Perception services (skeletons, haru-belief, haru-viz), plus adds a new recorder compose stack.

Changes:

  • Refactors Perception compose/services and env vars to use normalized topic namespaces and updated service set (adds skeletons, replaces people/visualization with haru-belief/haru-viz).
  • Adds a new recorder stack (apps/docker-compose-recorder.yaml + envs/recorder.env) and exposes it via scripts/compose.sh.
  • Updates documentation and startup commands to reflect the new Perception service lineup and web-based viz.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
start.sh Updates Perception start command (but currently out of sync with new compose services).
scripts/compose.sh Adds recorder stack support and adjusts formatting.
envs/recorder.env New environment file for recorder stack configuration.
envs/perception.env Updates ROS/FastDDS settings and introduces normalized topic/env conventions.
apps/docker-compose-recorder.yaml New compose stack for haru-recorder.
apps/docker-compose-perception.yaml Major Perception stack update: new services, normalized namespaces, new viz approach.
apps/docker-compose-all.yaml Updates all-in-one stack to include new Perception services.
README.md Updates pull/start instructions and expected Perception behavior.
.gitignore Adds Python cache artifacts to ignore list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread start.sh

# Perception services
bash scripts/compose.sh perception up azure-kinect faces hands people visualization --force-recreate -d
bash scripts/compose.sh perception up azure-kinect faces hands people --force-recreate -d
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

start.sh still tries to start the people service, but apps/docker-compose-perception.yaml no longer defines a people service (it has haru-belief/haru-viz/skeletons instead). This will make ./start.sh fail with "no such service". Update the perception up ... list to match the current perception stack services.

Suggested change
bash scripts/compose.sh perception up azure-kinect faces hands people --force-recreate -d
bash scripts/compose.sh perception up azure-kinect faces hands haru-belief haru-viz skeletons --force-recreate -d

Copilot uses AI. Check for mistakes.
Comment thread scripts/compose.sh Outdated
echo "Stacks: perception | speech | llm | reasoner | tts | simulator | all" >&2
exit 1
echo "Usage: $(basename "$0") <stack> <docker compose args...>" >&2
echo "Stacks: perception | speech | llm | reasoner | tts | simulator | all" >&2
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The usage help text doesn’t include the newly added recorder stack, even though it’s supported in the case statement. Update the "Stacks:" list so it matches the actual accepted values.

Suggested change
echo "Stacks: perception | speech | llm | reasoner | tts | simulator | all" >&2
echo "Stacks: perception | speech | llm | reasoner | tts | simulator | recorder | all" >&2

Copilot uses AI. Check for mistakes.
- ${XAUTHORITY:-$HOME/.Xauthority}:/root/.Xauthority:ro
- type: bind
source: ../../haru-viz
target: /workspace/haru-viz
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

haru-viz bind-mounts ../../haru-viz unconditionally. If that host path doesn’t exist (or is empty), Docker will create/mount an empty directory and mask whatever the image provides at /workspace/haru-viz, which can break startup. Consider making this mount opt-in (e.g., via a profile or an env-var controlled host path) or remove it for non-dev usage.

Suggested change
target: /workspace/haru-viz
target: /workspace/haru-viz
bind:
create_host_path: false

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +14
image: ${HARU_RECORDER_IMAGE:-ghcr.io/haru-project/haru-recorder:latest}
network_mode: *network-mode
env_file:
- ../envs/recorder.env
ports:
- "${HARU_RECORDER_WEB_UI_PORT:-8088}:${HARU_RECORDER_WEB_UI_PORT:-8088}"
volumes:
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

docker-compose-recorder.yaml sets network_mode to host by default, but also publishes ports. With network_mode: host, Docker Compose ignores ports: (and emits a warning), which can confuse users about how to reach the web UI. Either default COMPOSE_NETWORK_MODE to bridge for this stack, or document/remove the ports: mapping when running in host mode.

Copilot uses AI. Check for mistakes.
Comment thread README.md
docker pull ghcr.io/haru-project/strawberry-ros-hands:latest
docker pull ghcr.io/haru-project/haru-belief:feature-topic-normalize
docker pull ghcr.io/haru-project/haru-viz:develop
docker pull ghcr.io/haru-project/strawberry-ros-people:latest
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The install instructions still include pulling strawberry-ros-people:latest, but this PR removes the people service from the default perception compose stack (replaced by haru-belief). This makes the image pull list misleading; drop strawberry-ros-people here or explain when it’s still needed.

Suggested change
docker pull ghcr.io/haru-project/strawberry-ros-people:latest

Copilot uses AI. Check for mistakes.
Comment thread README.md
- The `haru-viz` web UI is reachable at `http://127.0.0.1:5173`
- rosbridge stays co-located with `haru-viz` and continues using ports `9090`, `9091`, and `9092`

**Related repositories for debug**: [strawberry-ros-people](https://github.com/haru-project/strawberry-ros-people/tree/ros2)
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

This section still points readers to strawberry-ros-people for perception-layer debugging, but the PR’s perception stack no longer runs the people service by default. Update the “Related repositories for debug” link to match the services users are actually running (e.g., haru-belief, haru-viz, strawberry-ros-skeletons, etc.).

Suggested change
**Related repositories for debug**: [strawberry-ros-people](https://github.com/haru-project/strawberry-ros-people/tree/ros2)
**Related repositories for debug**: [haru-belief](https://github.com/haru-project/haru-belief/tree/ros2), [haru-viz](https://github.com/haru-project/haru-viz), [strawberry-ros-skeletons](https://github.com/haru-project/strawberry-ros-skeletons/tree/ros2)

Copilot uses AI. Check for mistakes.
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