Project

General

Profile

Actions

Pull request #736

closed

Logger, surfaces and warnings

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

Status:
Closed
Priority:
Normal
Assignee:
Jules Waldhart

Description

In this pull request, GTP is switched entirely to use the logger.
moreover, now the surfaces draw handling is better (with a check button to enable/disable it)

The Ros interface also had gone through some modification and bugfixes.

The compilation warnings should all disapear from now, and as Jules said, we should from now on be sure to not introduce new ones.

Feature: now, in test file, one can specify an object placement.

available here:
/home/magharbi/new-pull-request/planners
/home/magharbi/new-pull-request/studio

both on branch master.

note: this should be done after: #753


Files

warning (3.82 KB) warning modified output of building in planners Jules Waldhart, 2016-03-01 17:39
Actions #1

Updated by Mamoun Gharbi about 8 years ago

Note: forgot to say but the connection with gtp_ros_bridge has also been updated to fit the use of solutionData.

Actions #2

Updated by Mamoun Gharbi about 8 years ago

  • Subject changed from Logger and surfaces to Logger, surfaces and warnings
  • Description updated (diff)
Actions #3

Updated by Jules Waldhart about 8 years ago

Some warnings I still have (attached)

Actions #4

Updated by Jules Waldhart about 8 years ago

  • Status changed from New to Feedback

in planners
--
commit "ae2c090e7167547 [GTP] Unknown TOASTER objects handling"
The message is not comprehensive enough, it should explain more whats going on and what may occur.

On the same line, you forgot the semicolon at the end of line, I don't get why it even compiles...

commit "f70983c888 [GTP] Switching GTP to the logger":
- use more hierarchy (e.g. gtp.tasks.manip.pick.multirrt)
- keep logger naming coherent (I used all lowercase, only alphanumeric char, but this can be discussed)

--
commit "e95351c3a61 [GTP] Adding object positioning function"
void HATPConnection::updateScenario(string rootStr)
rootStr should be named rootPath.

Plus, in that kind of situations, pass a reference! ( const string &rootPath )

--
Commit "6b37fb88 Remove warnings":
- put back localpath attribute in their original order in the class definition.
- keep tabulation consistency in files (e.g. in API/Roadmap/*)
- You removed stuff (e.g. in graph.cpp) but i'm not sure it should be removed. ( if (fct_stop != NULL) : does it compiles in MHP if you remove this ?) [1].
- In kinotrajectory.cpp:135, calling const method and ignoring return value, which is useless. (probably optimized out by the compiler)
- moveTo.cpp : new version is worse than previous.
- pickMultiRrt.cpp :
MANIPULATION_TASK_MESSAGE status = MANIPULATION_TASK_OK;
if (status == MANIPULATION_TASK_OK)

"if" statement is useless
- HRICS_FaceHumans.cpp : come on! How can you change a function(a whole class!!) to dummy just like that. Ok it's WIP, but put at least an error msg, or an abort()... [2]
- ConfGenerator.cpp : see [2]
- MultiHandOverUtils.cpp: It's maybe strictly personal, but I like to avoid using C-style comments (/**/), esp. in functions. (note: it's not personal: https://en.wikibooks.org/wiki/C%2B%2B_Programming/Code/Style_Conventions/Comments)

Morale de ce commit: supprimer les warnings n'est pas une fin en soit.


[1]: maybe its possible to remove the warning without loosing that functionality (if its called like a callback, it's probably because it can be a callback, and hence be NULL). The file where fct_stop and fct_draw are defined is under a compilation flag ( IF (NOT WITH_XFROMS ) in libmove3d/graphic/SourceList.cmake:18 )
[2]: we should discuss how to handle dummy functions in general

Actions #5

Updated by Jules Waldhart about 8 years ago

+ still a few cout in gtp (use grep -r cout src/GTP/ )

Actions #6

Updated by Mamoun Gharbi about 8 years ago

En ce qui concerne ce commit, et plus particulierement les tabulation et
les espaces, j'ai essayer de reparer, mais c'est vraiment pas la peine,
la plupart des fichiers sont deja un mix ...

on en rediscute, mais pour le moment je le laisse comme sa

Mamoun

On 03/02/2016 01:45 PM, Jules Waldhart wrote:

--
Commit "6b37fb88 Remove warnings":
- put back localpath attribute in their original order in the class definition.
- keep tabulation consistency in files (e.g. in API/Roadmap/*)

Actions #7

Updated by Mamoun Gharbi about 8 years ago

Comments of Jules taken into account

Actions #8

Updated by Jules Waldhart about 8 years ago

  • Status changed from Feedback to Closed
Actions

Also available in: Atom PDF