-
Notifications
You must be signed in to change notification settings - Fork 286
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
[Preliminary] Nodes and Addons #531
Merged
Merged
Changes from all commits
Commits
Show all changes
117 commits
Select commit
Hold shift + click to select a range
eeb6221
implementing Addon and AddonManager
mxgrey 3b35ee6
finished implementation of the AddonManager -- needs testing
mxgrey 87b3166
implementing State and Properties for AddonManager
mxgrey 0002f94
implemented Addon mechanics and writing tests
mxgrey dac3ae0
finished Addon & AddonManager implementation and testing
mxgrey 2e6b5c5
fixed documentation of macro
mxgrey 3a00af8
altering Node management -- strange indexing bug happening
mxgrey 29a11c0
fixed undefined behavior issue
mxgrey 40f3fd5
changed Addon State and Properties to use less dynamic allocation
mxgrey 9ea14a6
created Extensible class to give a common interface to all extensible…
mxgrey e20743d
store Nodes in a way that allows us to return vectors of Nodes by ref…
mxgrey 84170f2
add string header to Addon.h
mxgrey cbde581
created ExtendedProperties for BodyNode
mxgrey a1edd21
implemented ExtendedProperties setting
mxgrey 878c978
refactored Node semantics to be more extensible
mxgrey d9da5e9
ensuring that Nodes are copied over in the same order as the original
mxgrey cf511ab
making sure that Addons get cloned over
mxgrey 839ea29
pass over test that correctly triggers assertion in debug mode
mxgrey f9ed758
corrected const incorrectness
mxgrey b17d0a0
beginning changes to allow indexing Nodes within the Skeleton
mxgrey 468002e
generalizing Node name management
mxgrey 7137d08
Merge branch 'grey/analytical' into grey/addons
mxgrey a3d30de
patching up changes
mxgrey 4979c59
designing the machinery needed to specialize Nodes within Skeletons
mxgrey d01178f
created macros for specializing Nodes within Skeletons
mxgrey 5211a7d
implemented Skeleton-scope Node access -- needs testing
mxgrey d58690c
debugged Skeleton-scope Node access -- appears to be working
mxgrey e59beb3
swapped arguments for getting tree nodes
mxgrey 61f06c8
created test for generic nodes
mxgrey 86c976c
changed the Support class into an official Addon
mxgrey 521891c
merged in latest upstream changes
mxgrey a66fc66
made Extensibles easier to extend
mxgrey e7f6249
Merge branch 'grey/analytical' into grey/addons
mxgrey 62430bd
do not attempt to clone nullptrs
mxgrey 4134152
make analytical ik implementations clonable
mxgrey 52ad9e5
fixed undefined behavior
mxgrey 77e9b9c
transparency seems to work somewhat better without the transparency bin
mxgrey 7a6aa44
unprotect the typedef
mxgrey 00e5ec3
Merge branch 'grey/analytical' into grey/addons
mxgrey 6c46071
Merge branch 'grey/analytical' into grey/addons
mxgrey 06e39af
treat nullptr objective as a constant zero function
mxgrey 7da22af
Merge branch 'grey/urdf_world_parse_fix' into grey/addons
mxgrey 4ef6be7
Merge branch 'grey/urdf_world_parse_fix' into grey/addons
mxgrey 3c15845
replaced ugly macro with elegant template for Extensible mixins
mxgrey 418dc67
make sure that the ordering of cloned nodes will match the ordering o…
mxgrey cd0d25e
fixed recording toggle bug
mxgrey e98d37d
Merge branch 'master' of http://github.com/dartsim/dart into grey/addons
mxgrey 338d2c9
finished implementing extensions for EndEffector and tested it with Hubo
mxgrey 8425fc2
added Jetbrains directory to git ignore list
mxgrey 8b4d73f
removed debug printouts
mxgrey 10d5bfa
altered the default behavior for getNodeState and getNodeProperties
mxgrey 1a79f4a
removed the move semantics for Node::State and Node::Properties, beca…
mxgrey 1fcb734
adjusting fix for issue #499 to fit into the Node framework
mxgrey 72f5938
merged in better approach for tracking JacobianNodes
mxgrey 1ba3f8d
Merge branch 'grey/addons' of http://github.com/dartsim/dart into gre…
mxgrey 1b080b1
offering performant version of copying states and properties
mxgrey 33d77d7
implemented Skeleton configurations
mxgrey beaee49
fixed assignment typo bug
mxgrey 8461bc1
merged in changes for 5.1
mxgrey 09a60df
merged in latest analytical branch
mxgrey 38f2f39
merged in changes to IK modules
mxgrey 0e8261c
Merge remote-tracking branch 'origin/grey/analytical' into grey/addons
mxgrey 3f390da
Merge branch 'grey/analytical' into grey/addons
mxgrey 00310f3
making Skeleton::ExtendedProperties and convenience classes for creat…
mxgrey 2612a49
Merge branch 'grey/addons' of https://github.com/dartsim/dart into gr…
mxgrey 1ff2d54
renamed Addon::clone function to Addon::cloneAddon to avoid name coll…
mxgrey 517bf80
further implementing Addon convenience classes
mxgrey 3955285
defining Addon convenience classes
mxgrey 54a14f5
continuing to implement Addon convenience classes
mxgrey 1a50533
patching up some compilation errors
mxgrey 52dfb53
creating more direct accessor functions using templates
mxgrey dedb9ce
finished implementing Addon convenience classes
mxgrey 92108e9
including Skeleton in dynamics/Addon.h
mxgrey df23eab
using the specialized Skeleton Addon for the Support class
mxgrey 85d8954
beginning to implement Joint property addons
mxgrey 8660680
finished creating most-derived Joint property addons
mxgrey d711b28
created index-based set/get macro for Addon Properties
mxgrey 3ddcbff
creating Addon for SingleDofJoint properties
mxgrey c8d57be
implementing SingleDofJointAddon -- not finished
mxgrey dd7aabf
need to work out a circular dependency
mxgrey 7b6d6ef
finished all Joint Addons except MultiDofJoint
mxgrey 0007532
finished implementing all Joint addons
mxgrey 86c548e
merged in latest master changes
mxgrey 5ad56d2
changed const unique_ptr& to const Data& argument
mxgrey 03e7757
remove outdated comments
mxgrey 5448d04
changed const unique_ptr& to const Data& for Node State and Propertie…
mxgrey 6bf9bf2
corrected CRTP Addon implementations to use the new interface
mxgrey 0304a24
fixing clang compilation errors -- still need to fix linking errors
mxgrey a182162
making Clang happy
mxgrey 92b4b01
Merge branch 'grey/analytical' into grey/addons
mxgrey d2c3b95
implementing a CRTP method for specializing Addons
mxgrey 336c4c4
fixed typos
mxgrey 8f22d12
continuing CRTP implementation of Addon specialization
mxgrey 40eef30
fixed typos in implementation
mxgrey e6b81f2
updated test to use CRTP instead of macros
mxgrey 4dabe28
integrating Specialized Addon changes into the main codebase
mxgrey c34aa86
almost finished with implementation of SpecializedJoiner
mxgrey ee20d57
finished completely replacing the macro implementation of Addon speci…
mxgrey 2716e74
renaming Addon Specialization classes to avoid collisions with Node S…
mxgrey 747dd5c
beginning implementation of CRTP-based Node Specialization
mxgrey efd04d7
finished implementation of BasicNodeManager -- replaces macro-based s…
mxgrey 4669f72
removed the need for deferred_enable_if
mxgrey a343d3a
implemented NodeManagerJoiner classes
mxgrey fadb2ec
finished implementation of SpecializedNodeManagers
mxgrey c07f7db
finished implementation and integration of SpecializedNodeManagers
mxgrey 4ec1be8
Merge branch 'master' into grey/addons_merging_master
jslee02 2194f99
Make Visual Studio 2015 the minimum requirement
jslee02 b10b4a3
Use constexpr without checking compiler as Visual Studio 2015 became …
jslee02 bded530
Merge pull request #591 from dartsim/grey/addons_merging_master
jslee02 acfe8d2
making specialized managers virtual
mxgrey acf7573
Merge remote-tracking branch 'origin/master' into grey/addons_js_revi…
jslee02 4804229
Change note for developers to doxygen comment
jslee02 b329483
Remove _isSpecializedFor() from AddonManager
jslee02 b97d7f5
Do nothing when attempting to duplicate addons into itself
jslee02 668d334
Add missing constructor definition, and add move constructor to Joint…
jslee02 585cf4c
Add typename keyword to specify it's type name
jslee02 7b7a4b1
Merge pull request #598 from dartsim/grey/addons_js_revision
jslee02 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ TAGS | |
*.manifest | ||
*.rsp | ||
*~ | ||
.idea/* | ||
/nbproject/ | ||
/doxygen/html/ | ||
docs/readthedocs/site |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
/* | ||
* Copyright (c) 2015, Georgia Tech Research Corporation | ||
* All rights reserved. | ||
* | ||
* Author(s): Michael X. Grey <[email protected]> | ||
* | ||
* Georgia Tech Graphics Lab and Humanoid Robotics Lab | ||
* | ||
* Directed by Prof. C. Karen Liu and Prof. Mike Stilman | ||
* <[email protected]> <[email protected]> | ||
* | ||
* This file is provided under the following "BSD-style" License: | ||
* Redistribution and use in source and binary forms, with or | ||
* without modification, are permitted provided that the following | ||
* conditions are met: | ||
* * Redistributions of source code must retain the above copyright | ||
* notice, this list of conditions and the following disclaimer. | ||
* * Redistributions in binary form must reproduce the above | ||
* copyright notice, this list of conditions and the following | ||
* disclaimer in the documentation and/or other materials provided | ||
* with the distribution. | ||
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND | ||
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, | ||
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF | ||
* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE | ||
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR | ||
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, | ||
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT | ||
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF | ||
* USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED | ||
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | ||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN | ||
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE | ||
* POSSIBILITY OF SUCH DAMAGE. | ||
*/ | ||
|
||
#include <cassert> | ||
#include <string> | ||
#include <iostream> | ||
|
||
#include "dart/common/Addon.h" | ||
#include "dart/common/Console.h" | ||
|
||
namespace dart { | ||
namespace common { | ||
|
||
//============================================================================== | ||
void Addon::setAddonState(const State& /*otherState*/) | ||
{ | ||
// Do nothing | ||
} | ||
|
||
//============================================================================== | ||
const Addon::State* Addon::getAddonState() const | ||
{ | ||
return nullptr; | ||
} | ||
|
||
//============================================================================== | ||
void Addon::setAddonProperties(const Properties& /*someProperties*/) | ||
{ | ||
// Do nothing | ||
} | ||
|
||
//============================================================================== | ||
const Addon::Properties* Addon::getAddonProperties() const | ||
{ | ||
return nullptr; | ||
} | ||
|
||
//============================================================================== | ||
bool Addon::isOptional(AddonManager* /*oldManager*/) | ||
{ | ||
return true; | ||
} | ||
|
||
//============================================================================== | ||
Addon::Addon(AddonManager* manager) | ||
{ | ||
if(nullptr == manager) | ||
{ | ||
dterr << "[Addon::constructor] You are not allowed to construct an Addon " | ||
<< "outside of an AddonManager!\n"; | ||
assert(false); | ||
} | ||
} | ||
|
||
//============================================================================== | ||
void Addon::setManager(AddonManager* /*newManager*/, bool /*transfer*/) | ||
{ | ||
// Do nothing | ||
} | ||
|
||
} // namespace common | ||
} // namespace dart |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this function is for the legality check of removing from the manager, I think it should to be determined by its manager rather than by itself. If the decision is tied to the addon, we cannot create addons of the addon type from two managers, where one is general manager (or specialized manager but not for the addon) and the other one is a specialized manager for the addon, expecting one is specialized and the other is not specialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my original plan, but I had a very hard time thinking of a clean way to implement that. I found it much easier to have that information embedded in the
Addon
rather than trying to get it into theSpecializedAddonManager
.It should be noted that this function provides the
Addon
with a pointer to its manager, so theAddon
can determine whether it's required for this particular type of manager. So if we have aFooAddon
that's "required" only for aFooManager
but we attach it to aGeneralManager
, then the functionFooAddon::isOptional(AddonManager* mgr)
can dynamic_cast themgr
argument to aFooManager*
. If the dynamic_cast fails, thenFooAddon::isOptional
will returntrue
, if the cast succeeds then it will returnfalse
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'm not sure if we really need this function. (If there wouldn't be any additional use case,) It's currently used in AddonManager and SpecializedManager to check the legality of removing from the managers. In AddonManager, we might don't need it since all the addons in AddonManager are not specialized. For SpecializedAddonManager, we can check the legality in more simple way. For example,
erase()
function can be implemented as:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is, not all specialized addons are required. For example, for
ShapeNode
, we will have aVisualData
and aCollisionData
specialized addon, but neither of them are required to be there, because their absence just means that the ShapeNode doesn't want to visualize or doesn't want to collide. On the other hand,RevoluteJoint
has aRevoluteJointAddon
which contains axis properties, and that addon is required because the dynamics and kinematics computations can't work without it.A specialized addon just allows us to have constant-time access to it, and does not imply that the addon is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense, but I wonder if there is a use case that the optionality should be checked from addon instead of from the specific manager we want to check. We can check it from
FooManager
andGeneralManager
forFooAddon
for the above case.Even though there are use cases,
isOptional()
doesn't seem to work as we expect. For example,AddonWithProtectedPropertiesInSkeleton::isOptional()
would always return true regardless the passed in manager type if true is passed in for the template parameterOptionalT
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as whether I expect the behavior of (1) or (2) it will depend on whether we have
MandatoryAddon
also specialize the addon. I think we should, because why not? If an addon is going to be mandatory, we may as well specialize it as well, because clearly we expect it to exist.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like the map can have any addons regardless of each addon is optional or mandatory. Hm, I'm more confused, haha. Here is my understanding. Please let me know which one is different from you.
In that sense, it seems the mandatory addons should be a subset of specialized addons. So I stored the optionality value within specialized addon map (that is stored in the basic addon manager) in the code, and worried the attemptions to set the optionality when the associated addon hasn't added yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really agree with the subset relationship. Here's how I think it should work:
These are two completely independent concepts. The only point at which they overlap is that if an addon is mandatory for a certain manager type, then we may as well also specialize the manager for that addon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point. What I'm not clear is why we open the case that an addon is mandatory but not specialized. Wouldn't it be too strong restriction if we enforce the user to use specialized manager for every mandatory addons so that the user can always take the advantage of the zero-overhaed access to that addon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I agree. I think the concepts of specialized and mandatory are inherently independent, but if an addon is important enough to be mandatory, then we may as well have the API also specialize it, especially since it would be as easy as having
MandatoryAddon
virtually inheritSpecializedAddonManager
.