Project

General

Profile

Actions
Copy link

Pull request #690

closed

[all] Adding parsing JSON files for GTP to use

Added by Mamoun Gharbi about 9 years ago. Updated about 9 years ago.

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)

Actions
Copy link
 #1

Updated by Jules Waldhart about 9 years ago

  • Status changed from New to In Progress
  • Assignee set to Jules Waldhart

Adding assignee and watchers!!!

Actions
Copy link
 #2

Updated by Jules Waldhart about 9 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.

Actions
Copy link
 #3

Updated by Mamoun Gharbi about 9 years ago

The suggestion were taken into account

Actions
Copy link
 #4

Updated by Renaud Viry about 9 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.

Actions
Copy link
 #5

Updated by Mamoun Gharbi about 9 years ago

  • Status changed from In Progress to Resolved

pushed to redmine

Actions
Copy link
 #6

Updated by Jules Waldhart about 9 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF