Problem::check() more exhaustive

this function is not checking if stateblocks are not nullptr. Independently of verbose_level value, all the state blocks should be checked to be valid.

Also, it would be nice if the problem is not consistent, raising a message explaining the reason apart from updating the variable is_consistent.

And finally, instead of iterating over getStateBlockVec(), iterate over getStateBlockMap() and then the key (string) of each state block can be printed as well.

changed the description

assigned to @jcasals

created merge request !340 (merged) to address this issue

mentioned in merge request !340 (merged)

What was implemented in the end?

It's a WIP, you can see the changes at !340 (merged)

I guess this part

And finally, instead of iterating over getStateBlockVec(), iterate over getStateBlockMap()
and then the key (string) of each state block can be printed as well.

also applies to Problem::print() right? @joanvallve So I just make the modification in both functions

I think this is already done in Problem::print()

Maybe, we can benefit from some renamings that would make the check message more readable, especially the processor, sensors, frames, captures...
I suggest replacing single letters by 3 letters abreviations. If find the uppercase/lowercase meanings are kinda hard to remember, let alone to figure out the first time. For instance:
c -> Fac, C4 -> Cap, f -> Fea, F -> Fra, KF -> KFra, P -> Pro ...

I don't have a particular opinion on this matter. If @jsola @joanvallve are fine with it I'll go ahead and do it.

I am not fond of the "--> C" symbolizing the presence of a capture other in the factor as well, something more verbose like "--> CapOther" would be cool. Same for Frame/Feature/Landmark other.

I agree on Mederic's proposal

OK with one condition: Labels are of length 3, with one decorator.

That is, labels are the WOLF nodes:

Prb, Hrw, Trj, Map, Sen, Prc, Frm, Cap, Ftr, Fac, Lmk

And decorators are, e.g. K , M, T

• KFrm : keyframe
• CapM : capture Motion
• PrcM : Processor motion
• PrcT : Processor tracker
• PrcL : Processor loop closure

This is so to be able to respect the tabulations as much as possible. Otherwise reading these check() and print() things becomes very difficult. For this, I suggest the decorations to span only one letter and not three more letters -- but other opinions are welcome.

That looks good to me, let's implement that. There is also a problem line 1767 with the indentation of " --> C" @jsola was mentioning. But it seemed to be problematic wasn't it?

In fact, and since we are here, let me say that the functions print() and check() are horrible.

What we would need is a couple of viral functions print() and check() that:

• print the current node
• call print() on all child nodes

Moreover, each node's print would be virtual, so that derived classes can print more info / modified info , with respect to the base class.

A typical print() function in a node Parent, with children of type Child, would be split into a header section and a viral section; The Header section can be derived and so it is virtual:

virtual ParentBase::printHeader()
{
  // print relevant info of ParentBase
  // ...
}

ParentBase::print()
{
  printHeader();
  for (const ChildBasePtr& child : child_list_)
  {
     child->print();
  }
}

virtual ParentDerived::printHeader(options)
{
  FooBase::printHeader(); // print base header

  // print relevant info of ParentDerived
  // ...
}

At this point, if all Nodes in wolf have these, we can call

problem->print(options);

and obtain the same behavior as now.

closed via merge request !340 (merged)

mentioned in commit 23572ae6

mentioned in issue #296 (closed)