Skip to content

Expose TTC computation to esminiJS API#824

Open
kosuke55 wants to merge 6 commits into
esmini:devfrom
kosuke55:feat/esminijs-ttc
Open

Expose TTC computation to esminiJS API#824
kosuke55 wants to merge 6 commits into
esmini:devfrom
kosuke55:feat/esminijs-ttc

Conversation

@kosuke55

Copy link
Copy Markdown
Contributor

Follow-up to #821, which added TTC to the esminiLib C API.

This adds the same Object::TimeToCollision computation to the esminiJS (WASM)
API, so it can be called from the browser build via get_ttc.

@kosuke55
kosuke55 changed the base branch from master to dev July 19, 2026 02:12
@kosuke55
kosuke55 force-pushed the feat/esminijs-ttc branch from 7f72004 to 481cc0f Compare July 19, 2026 02:19
@kosuke55

Copy link
Copy Markdown
Contributor Author

@johschmitz
Since this touches the esminiJS bindings you built in #814, would you be able to take a look when you have a moment? It just exposes the TTC computation (added in #821) through get_ttc. Thanks!

return copy_state_from_object(object, scenario_engine_->getSimulationTime());
}

double OpenScenario::get_ttc(int object_a_id, int object_b_id, bool free_space, int cs, int dist_type) const

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For consistency with the rest of the codebase I suggest time_to_collision() since
I understand this is a wrapper for TimeToCollision()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks, that is better.
Renamed to get_time_to_collision() keeping the get_ prefix to match the other esminiJS getters.
a3dfb0d

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not 100% sure how consistent this is being handled in this project (maybe we need to cross check) but according to my most common understanding getters just access a value of a single object while this method is performing calculations across two object's data. Hence, IMHO for this particular method the get_ is somewhat misleading/confusing (unless some other code constantly calculates all TTCs towards all other objects and then stores a huge array of these values alongside every object and that is what we are accessing here).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, agreed. Not a pure getter, so dropped get_.
a7caf64

Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>

scenarioengine::Object* obj_a = scenario_engine_->entities_.GetObjectById(object_a_id);
scenarioengine::Object* obj_b = scenario_engine_->entities_.GetObjectById(object_b_id);
if (obj_a == nullptr || obj_b == nullptr)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lots of return -1 extra checks. I think not all of these checks are needed, some of it is handled inside the wrapped method and maybe some conditions can never occur. IMHO add necessary checks to avoid crashes but other than that try to make the wrapper as thin as possible and if necessary add additional checks to the wrapped method so that WASM ans non-WASM builds behave as similar as possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

made it thinner and kept only the null checks needed to avoid a crash.
dc9b1a4

kosuke55 added 4 commits July 19, 2026 16:06
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
Signed-off-by: kosuke55 <kosuke.tnp@gmail.com>
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.

2 participants