-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Important modifications #116
Comments
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 ? |
We should not make a release until I make the PR and they are merged. The current devel branch is not good. |
Ok, there was a conflict in your message between:
And
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 ? |
I do not think it is necessary. Only
|
I need one to workaround some breaks introduced by the latest eigenpy release |
I would recommend to use |
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:
DynamicPinocchio
that reads a URDF string fromrobot_description
parameter,Device
that reads some ROS parameter to known which joints areactuated 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 notyet 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 aresult, 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,
typedef dynamicgraph::size_type
asEigen
Index
type,typedef dynamicgraph::int64_t sigtime_t
,int
tosize_type
,unsigned int
tostd::size_t
,SignalPtr
class: it is not anymore possible to plug aSignal<A>
into aSignalPtr<B>
if B is not A (formerly, A couldderive from B). This feature is not anymore used.
The text was updated successfully, but these errors were encountered: