Actions
Pull request #690
closed[all] Adding parsing JSON files for GTP to use
Status:
Closed
Priority:
Normal
Assignee:
Jules Waldhart
Description
for the surfaces, the object rotation, to define which are the agents and which are the virtual objects, and what are the objects with a vertical constraint.
The repositories are in /home/magharbi/pull-request/
there is:
libmove3d
libmove3d-hri
libmove3d-planners
move3d-studio
data
(all on branch master)
Updated by Jules Waldhart over 8 years ago
- Status changed from New to In Progress
- Assignee set to Jules Waldhart
Adding assignee and watchers!!!
Updated by Jules Waldhart over 8 years ago
Suggestions & remarks:
- squash the Bugfix with its parent commit
- remove useless comments in planners/.../objectrob.*
- remove useless redundant Scene::doesRobotExist(string str). Can use Scene::getRobotByName(string str) exactly the same way.
- useless call to p3d_vec3Mat4Mult(src,M,res) in GeometricForm::transformNorm(p3d_vector3 src, p3d_matrix4 M, p3d_vector3 res)
- comment difference between transformPoint and transformNorm
- have to check/discuss the change in p3d_plan(p3d_rob *robotPt, configPt from, configPt to, const std::vector<int>...)
- give 1/2-line comment for each class in Entities, about what data they manage.
- coherence: EntityManager::readJson() should be called fetchData()
- +1 for using logmove3d :)
- +1 tracking errors in Entities looks ok (easy to find erroneous/missing file according to log messages. didn't check all possible cases, hope it's all coherent)
So it's a "conditional accept" for me.
Updated by Mamoun Gharbi over 8 years ago
The suggestion were taken into account
Updated by Renaud Viry over 8 years ago
Agreed with remarks from Jules.
Also, ran some tests on GTP (and a few on basic motion planning) and works fine.
Same conditionnal accept for me.
Updated by Mamoun Gharbi over 8 years ago
- Status changed from In Progress to Resolved
pushed to redmine
Updated by Jules Waldhart over 8 years ago
- Status changed from Resolved to Closed
Actions