HITL: rearrange_v2 example app and other features/fixes for Feb 12 demo #1810

eundersander · 2024-02-15T21:54:00Z

Motivation and Context

A big chunk of changes made in the last week leading up to our Feb 12/13 demo:

nicer instructions for installing Magnum on Mac
add MVP rearrange_v2 HITL app
add GuiPlacementHelper
add ClientHelper
changes to GuiPickHelper.
add config habitat_hitl.debug_line_width
changes to how connection_record/connection_params are handled
add config habitat_hitl.networking.wait_for_app_ready_signal
support for tracking client latency
add color and billboard-versus-up-facing options for highlight messages
minor bugfix to unity_dataset_processing.py
add hydra-core to requirements.txt

How Has This Been Tested

Some local testing with the various HITL apps.

Todo:

figure out why basic_viewer agents stopped moving (wrong config setting during my testing)
test Pick_throw_vr with ~~the VR client~~ Unity VR emulation

Types of changes

[HITL]

Checklist

My code follows the code style of this project.
I have updated the documentation if required.
I have read the CONTRIBUTING document.
I have completed my CLA (see CONTRIBUTING)
I have added tests to cover my changes if required.

* support for processing new client connections, including the "connection record" * changed client-ready message to instead be the connection record; see stub_client.html for reference * RemoteGuiInput logic to clear input history when client changes (i.e. disconnect/reconnect)"

…n polish

…PEN_CLOSE feature flag

… for rearrange_v2; add ESC to AgnosticKeyNS; mypy/flake8 fixes

…ed objects.

eundersander

Some comments for reviewers.

eundersander · 2024-02-15T23:38:31Z

habitat-hitl/habitat_hitl/_internal/hitl_driver.py

@@ -450,15 +449,6 @@ def sim_update(self, dt):
            + dt * self._hitl_config.viz_animation_speed
        ) % 1.0

-        # Navmesh visualization only works in the debug third-person view


This kind of behavior tied to a keypress should be implemented in AppStates, not HitlDriver.

eundersander · 2024-02-15T23:39:08Z

habitat-hitl/habitat_hitl/_internal/hitl_driver.py

@@ -475,13 +465,6 @@ def sim_update(self, dt):
            for keyframe in keyframes:
                self._recording_keyframes.append(keyframe)

-        # Manually save recorded gfx-replay keyframes.


This looks like debug code. Removing it. It can be re-added to individual AppStates that need it.

eundersander · 2024-02-15T23:42:05Z

habitat-hitl/habitat_hitl/environment/gui_pick_helper.py

@@ -10,6 +10,12 @@
 import numpy as np

 DIST_HIGHLIGHT: Final[float] = 0.15
+COLOR_GRASPABLE: Final[mn.Color3] = mn.Color3(1, 0.75, 0)


This class was not being used at all in main. I made some major changes to get it to work for rearrange_v2.

0mdc

Nice progress! I did a first pass and left some comments.

0mdc · 2024-02-16T21:28:59Z

habitat-hitl/habitat_hitl/core/gui_input.py

+
+    # physical key enum from USB HID Usage Tables
+    # https://www.usb.org/sites/default/files/documents/hut1_12v2.pdf, page 53
+    # add misisng keys here as necessary


Typo: misisng

0mdc · 2024-02-16T22:21:17Z

habitat-hitl/habitat_hitl/core/client_message_manager.py

-        )
+        highlight_dict = {"t": [pos[0], pos[1], pos[2]], "r": radius}
+        if billboard:
+            highlight_dict["b"] = 1


Billboarding could be driven by the server.

To be clear, I'm not suggesting any change here, the billboard parameter makes it easy to replace.

I suggest doing billboarding on the client. In general, I don't always expect the server to drive the camera.

0mdc · 2024-02-16T22:22:04Z

habitat-hitl/habitat_hitl/core/client_message_manager.py

+
+            def conv(channel):
+                # sloppy: using int 0-255 to reduce serialized data size
+                return int(channel * 255.0)


0mdc · 2024-02-16T22:29:03Z

habitat-hitl/habitat_hitl/core/text_drawer.py

    def add_text(
        self,
        text_to_add,
        alignment: TextOnScreenAlignment = TextOnScreenAlignment.TOP_LEFT,
        text_delta_x: int = 0,
        text_delta_y: int = 0,
    ):
-        pass
+        align_y, align_x = alignment.value


Unrelated: I find TextOnScreenAlignment being {y, x} hard to follow. It's probably worth a comment, rename or swap.

Agreed. The text layout system needs a large refactor at some point.

0mdc · 2024-02-16T22:31:29Z

habitat-hitl/habitat_hitl/environment/gui_placement_helper.py

+        self._app_service = gui_service
+        self._gravity_dir = gravity_dir
+
+    def _find_snap_pos(self, ray, query_obj):


Type annotations would be useful on this one because the return type is non-trivial.

0mdc · 2024-02-16T22:37:39Z

examples/hitl/rearrange_v2/rearrange_v2.py

+            self._gui_agent_ctrl._humanoid_controller.obj_transform_base.translation
+        )
+
+    def _get_gui_agent_feet_height(self):


Same here, I would move this in GuiHumanoidController.

0mdc · 2024-02-16T22:40:35Z

examples/hitl/rearrange_v2/rearrange_v2.py

+                )
+                self._app_service.end_episode(do_reset=True)
+
+    def _update_placement(self):


I would rename this to something along the lines of _update_held_object_placement().

0mdc · 2024-02-16T22:44:30Z

examples/hitl/rearrange_v2/rearrange_v2.py

+
+        if self._placement_helper.update(ray, self._held_obj_id):
+            # sloppy: save another keyframe here since we just moved the held object
+            self.get_sim().gfx_replay_manager.save_keyframe()


Interesting. Why is this required here?

If we don't add this, we see a lag between the camera movement and the snapped held object placement.

This is essentially part of a "post update" (after the underlying sim step). The sim step updates the agent pose. We update the camera from the updated agent pose, then we update the held object placement from the camera (since it uses a screen-relative raycast).

A gfx-replay keyframe is captured inside the sim step, but in this case we're manipulating the scene after the sim step.

0mdc · 2024-02-16T22:54:10Z

habitat-hitl/habitat_hitl/environment/gui_placement_helper.py

+        self._app_service = gui_service
+        self._gravity_dir = gravity_dir
+
+    def _find_snap_pos(self, ray, query_obj):


I would rename this function to something like _try_snap_object_to_surface to convey that this applies a translation.

0mdc · 2024-02-16T23:01:15Z

examples/hitl/rearrange_v2/rearrange_v2.py

+    def is_user_idle_this_frame(self):
+        return not self._app_service.gui_input.get_any_key_down()
+
+    def _update_for_remote_client_connect_and_idle(self):


It may be clearer to split this function into connection handling, idle timer handling and sps counter updating.

polish from PR feedback

0mdc

Looks good! I think that this is a good starting point for rearrange_v2.

0mdc · 2024-02-20T22:10:57Z

habitat-hitl/habitat_hitl/core/client_helper.py

+class ClientHelper:
+    """
+    Tracks connected remote clients. Displays client latency and kicks idle clients.
+    """


I like that we're grouping this functionality here.

Nit: the name ClientHelper is vague in terms of what this class actually does.

Yeah but I figure the functionality will grow.

…mo (#1810) * nicer instructions for installing Magnum on Mac * add MVP rearrange_v2 HITL app * add GuiPlacementHelper * add ClientHelper * changes to GuiPickHelper. * add config habitat_hitl.debug_line_width * changes to how connection_record/connection_params are handled * add config habitat_hitl.networking.wait_for_app_ready_signal * support for tracking client latency * add color and billboard-versus-up-facing options for highlight messages * minor bugfix to unity_dataset_processing.py * add hydra-core to requirements.txt

…mo (facebookresearch#1810) * nicer instructions for installing Magnum on Mac * add MVP rearrange_v2 HITL app * add GuiPlacementHelper * add ClientHelper * changes to GuiPickHelper. * add config habitat_hitl.debug_line_width * changes to how connection_record/connection_params are handled * add config habitat_hitl.networking.wait_for_app_ready_signal * support for tracking client latency * add color and billboard-versus-up-facing options for highlight messages * minor bugfix to unity_dataset_processing.py * add hydra-core to requirements.txt

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 15, 2024

eundersander force-pushed the eundersander/hitl-demo-feb12-refactored branch from 8e91e59 to 29c3abc Compare February 15, 2024 22:38

eundersander and others added 28 commits February 15, 2024 17:22

Mac Unity data processing instructions

f2ee43d

add habitat_hitl.debug_line_width config

aab6031

remove str usage because it caused a runtime error for me

43e3325

make server robust to empty connection_params; some black formatting

f658e2d

Rearrange_v2 HITL app, initial version

e43720d

remove some debug code that shouldn't go in hitl_driver

aced830

Make GuiInput keys work with headless mode

a218eb6

Rearrange v2 polish

5c58615

fix TextDrawer for headless mode

085fd77

headless server logging polish

a822369

print idle kick warning

26c0449

Activate scene mesh decimation in unity dataset processing.

5ac13c6

bugfix for headed GuiInput; reduce http server log spam; documentatio…

8b92a57

…n polish

add help text for server SPS and latency

9432431

sloppy attempt to fix some connection issues

203f89b

stub client polish

a5d6580

add hydra-core to requirements

aa0b0af

add missing average_helper.py

8ba6b0b

add articulated open/close support hidden behind ENABLE_ARTICULATED_O…

98c7d6d

…PEN_CLOSE feature flag

Change rearrange_v2 config to use demo episode set.

379be35

Add demo documentation.

5b609d7

Upload rearrange_v2 episodes. Update README.md.

fa50c78

polish how application_exit is handled; add hotkeys to change episode…

a1095bd

… for rearrange_v2; add ESC to AgnosticKeyNS; mypy/flake8 fixes

add GuiPlacementHelper and integrate into rearrange_v2

aa250bf

config changes to disable spot picking/grasping and backing up

ea5b475

Change episode set so that the second scene doesn't contain articulat…

9ca77ec

…ed objects.

Update data folder in README.

ab3e0d4

eundersander added 9 commits February 15, 2024 17:27

episode-selection polish

552dd5d

highlight ring polish

a4d09aa

revert to old episode dataset

5149421

switch to new articulated scenes

f596b6b

rearrange_v2 help text polish; precommit fixes

c7a0083

undo some recent changes to unity_dataset_processing.py

abd1773

remove some commented-out code

9e8a91d

move demo.json.gz to rearrange_v2/app_data folder

ddd0180

fix after rebase

666bc0b

eundersander force-pushed the eundersander/hitl-demo-feb12-refactored branch from 52a00fa to 666bc0b Compare February 15, 2024 23:36

eundersander commented Feb 15, 2024

View reviewed changes

eundersander requested a review from 0mdc February 15, 2024 23:46

0mdc reviewed Feb 16, 2024

View reviewed changes

This was referenced Feb 16, 2024

Add keyboard input tracking. eundersander/siro_hitl_unity_client#28

Closed

Add keyboard input tracking. 0mdc/siro_hitl_unity_client#3

Merged

Add mouse input tracking. 0mdc/siro_hitl_unity_client#5

Merged

add ClientHelper

82d7f2f

polish from PR feedback

eundersander marked this pull request as ready for review February 20, 2024 15:39

workaround for spurious mypy errors

3e76c35

0mdc approved these changes Feb 20, 2024

View reviewed changes

eundersander merged commit 03de61e into main Feb 20, 2024
4 checks passed

eundersander deleted the eundersander/hitl-demo-feb12-refactored branch February 20, 2024 23:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HITL: rearrange_v2 example app and other features/fixes for Feb 12 demo #1810

HITL: rearrange_v2 example app and other features/fixes for Feb 12 demo #1810

eundersander commented Feb 15, 2024 •

edited

Loading

eundersander left a comment

eundersander Feb 15, 2024

eundersander Feb 15, 2024

eundersander Feb 15, 2024

0mdc left a comment

0mdc Feb 16, 2024

0mdc Feb 16, 2024

eundersander Feb 20, 2024

0mdc Feb 16, 2024

0mdc Feb 16, 2024

eundersander Feb 20, 2024

0mdc Feb 16, 2024

0mdc Feb 16, 2024

0mdc Feb 16, 2024

0mdc Feb 16, 2024

eundersander Feb 20, 2024

0mdc Feb 16, 2024

0mdc Feb 16, 2024 •

edited

Loading

0mdc left a comment

0mdc Feb 20, 2024

eundersander Feb 20, 2024

HITL: rearrange_v2 example app and other features/fixes for Feb 12 demo #1810

HITL: rearrange_v2 example app and other features/fixes for Feb 12 demo #1810

Conversation

eundersander commented Feb 15, 2024 • edited Loading

Motivation and Context

How Has This Been Tested

Types of changes

Checklist

eundersander left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0mdc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0mdc Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

0mdc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eundersander commented Feb 15, 2024 •

edited

Loading

0mdc Feb 16, 2024 •

edited

Loading