Important modifications

[florent-lamiraux]

I open this issue in this package since this is the upstream package, but the issue concerns the following packages:

• dynamic-graph,
• dynamic-graph-python,
• sot-core,
• sot-dynamic-pinocchio,
• dynamic_graph_bridge,
• roscontrol_sot,
• sot-universal-robot.

I have made important modifications in these packages that I will submit in several pull requests and I suggest to include these modifications in a version 5 of the software.

Motivation and first modifications

The first motivation came from the observation that initializing a robot in a ROS environment was difficult since two models of the robot coexist:

• one in DynamicPinocchio that reads a URDF string from robot_description parameter,
• one in Device that reads some ROS parameter to known which joints are actuated via roscontrol.

The two models do not necessarily have the same dimension. For example, Franka robot have 7 joints controlled via roscontrol and 2 gripper joints controlled via ros actions.

Following a long discussion among users of the software, and in order to make initialization more general, I first extracted the integration of the velocity from the Device and created a special entity called Integrator.

The second motivation came from the observation that on some robots, the control loop is not regularly called as it should be, but the time since last call is passed to the controller (https://github.com/stack-of-tasks/sot-core/blob/d49cab80bc8160dcb9da10afdd6a8e1f51e37553/include/sot/core/abstract-sot-external-interface.hh#L59).

The problem was to pass this time to the new integrator. I decided to use the signal time to do so, converting the period into microseconds. Between two evaluations of the integrator output, the time is now incremented by a number much bigger than 1 (as previously).

The modifications described here are merged in the devel branches, but not yet released. I think we should not release them.

Further modifications

Then I realized that the type for signal time (int) is encoded on 32bits. I naively thought that the size of int had changed to 64bits on modern CPUs. As a result, everything worked fine for 35 minutes and then the time was overflown.

To fix the overflow issue, I then changed the signal type into long int (or int64_t). This change modifies most of the API and I think we should switch to version 5 since the overall modifications is important.

Mainly,

• I defined a typedef dynamicgraph::size_type as Eigen Index type,
• I defined a typedef dynamicgraph::int64_t sigtime_t,
• I changed most occurences of int to size_type,
• I changed most occurences of unsigned int to std::size_t,
• I simplified SignalPtr class: it is not anymore possible to plug a Signal<A> into a SignalPtr<B> if B is not A (formerly, A could derive from B). This feature is not anymore used.

[nim65s]

Ok, thanks for this detailed explanation.

You are suggesting to bump the major version number of the project in the next release, but at the same time you think we should not make a release. What are we waiting for exactly ?

[florent-lamiraux]

We should not make a release until I make the PR and they are merged. The current devel branch is not good.

[nim65s]

Ok, there was a conflict in your message between:

I will submit in several pull requests

And

are merged in the devel branches

And I parsed that in the wrong way. Now this is more clear.

Following question: Could we make a non-breaking latest release before we merge the PR you are describing here ?

Also, could you link those breaking PR to this issue, so we can easily track them ?

[florent-lamiraux]

Following question: Could we make a non-breaking latest release before we merge the PR you are describing here ?

I do not think it is necessary. Only

• 9d8ba7f1 in dynamic-graph could be useful, but it does not deserve a full release.

[nim65s]

I need one to workaround some breaks introduced by the latest eigenpy release

[olivier-stasse]

I would recommend to use uint64_t instead of unsigned long which is more ambiguous.