diff --git a/doc/XMLreference.rst b/doc/XMLreference.rst index 002bb425..66420bcb 100644 --- a/doc/XMLreference.rst +++ b/doc/XMLreference.rst @@ -47,6 +47,8 @@ in the second column of the table have the following meaning: | | | +-------------------------+-------------------------+-------------------------+ | | | | | :at:`inertiafromgeom` | :at:`inertiagrouprange` | :at:`exactmeshinertia` | | | | | +-------------------------+-------------------------+-------------------------+ | +| | | | :at:`autolimits` | | | | +| | | +-------------------------+-------------------------+-------------------------+ | +--------------------------+----+------------------------------------------------------------------------------------+ | |_2|:el:`lengthrange` | ? | .. table:: | | | | :class: mjcf-attributes | @@ -1524,6 +1526,13 @@ The unique top-level element, identifying the XML file as an MJCF model file. This element is used to set options for the built-in parser and compiler. After parsing and compilation it no longer has any effect. The settings here are global and apply to the entire model. +:at:`autolimits`: :at-val:`[false, true], "false"` + This attribute affects the behavior of attributes such as "limited" (on or ), "forcelimited", + "ctrllimited", and "actlimited" (on ). If "true", these attributes are unnecessary and their value + will be inferred from the presence of their corresponding "range" attribute. + If "false", no such inference will happen: For a joint to be limited, both limited="true" and range="min max" must + be specified. In this mode, it is an error to specify a range without a limit. + |br| The default for this option will be set to "true" in an upcoming release. :at:`boundmass`: :at-val:`real, "0"` This attribute imposes a lower bound on the mass of each body except for the world body. Setting this attribute to a value greater than 0 can be used as a quick fix for poorly designed models that contain massless moving bodies, @@ -3162,7 +3171,8 @@ unit quaternions. :at:`limited`: :at-val:`[false, true, auto], "auto"` This attribute specifies if the joint has limits. It interacts with the range attribute below. If this attribute is "false", joint limits are disabled. If this attribute is "true", joint limits are enabled. If this - attribute is "auto", joint limits will be enabled if range is defined and disabled otherwise. + attribute is "auto", and :at:`autolimits` is set in :ref:`compiler `, joint limits will be enabled + if range is defined. :at:`solreflimit`, :at:`solimplimit` Constraint solver parameters for simulating joint limits. See :ref:`CSolver`. :at:`solreffriction`, :at:`solimpfriction` @@ -3176,6 +3186,8 @@ unit quaternions. joints, the limit is imposed on the angle of rotation (relative to the reference configuration) regardless of the axis of rotation. Only the second range parameter is used for ball joints; the first range parameter should be set to 0. See the :ref:`Limit ` section in the Computation chapter for more information. + |br| Setting this attribute without specifying :at:`limited` is an error, unless :at:`autolimits` is set in + :ref:`compiler `. :at:`margin`: :at-val:`real, "0"` The distance threshold below which limits become active. Recall that the :ref:`Constraint solver ` normally generates forces as soon as a constraint becomes active, even if the margin parameter makes that happen at a @@ -4122,10 +4134,11 @@ the obstacle geom. visualizer to enable and disable the rendering of entire groups of tendons. :at:`limited`: :at-val:`[false, true, auto], "auto"` If this attribute is "true", the length limits defined by the range attribute below are imposed by the constraint - solver. If this attribute is "auto", length limits will be enabled if range is defined and disabled otherwise. + solver. If this attribute is "auto", and :at:`autolimits` is set in :ref:`compiler `, length limits + will be enabled if range is defined. :at:`range`: :at-val:`real(2), "0 0"` - Range of allowed tendon lengths. To enable length limits, set the limited attribute to "true" in addition to defining - the present value. + Range of allowed tendon lengths. Setting this attribute without specifying :at:`limited` is an error, unless + :at:`autolimits` is set in :ref:`compiler `. :at:`solreflimit`, :at:`solimplimit` Constraint solver parameters for simulating tendon limits. See :ref:`CSolver`. :at:`solreffriction`, :at:`solimpfriction` @@ -4269,25 +4282,33 @@ specify them independently. visualizer to enable and disable the rendering of entire groups of actuators. :at:`ctrllimited`: :at-val:`[false, true, auto], "auto"` If true, the control input to this actuator is automatically clamped to :at:`ctrlrange` at runtime. If false, control - input clamping is disabled. If auto, control clamping will automatically be set to true if :at:`ctrlrange` is - defined without explicitly setting this attribute to "true". Note that control input clamping can also be globally - disabled with the :at:`clampctrl` attribute of :ref:`option/flag `. + input clamping is disabled. If "auto" and :at:`autolimits` is set in :ref:`compiler `, control + clamping will automatically be set to true if :at:`ctrlrange` is defined without explicitly setting this attribute + to "true". Note that control input clamping can also be globally disabled with the :at:`clampctrl` attribute of + :ref:`option/flag `. :at:`forcelimited`: :at-val:`[false, true, auto], "auto"` If true, the force output of this actuator is automatically clamped to :at:`forcerange` at runtime. If false, force - clamping is disabled. If auto, force clamping will automatically be set to true if :at:`forcerange` is - defined without explicitly setting this attribute to "true". + clamping is disabled. If "auto" and :at:`autolimits` is set in :ref:`compiler `, force clamping will + automatically be set to true if :at:`forcerange` is defined without explicitly setting this attribute to "true". :at:`actlimited`: :at-val:`[false, true, auto], "auto"` If true, the internal state (activation) associated with this actuator is automatically clamped to :at:`actrange` at - runtime. If false, activation clamping is disabled. If auto, activation clamping will automatically be set to true - if :at:`actrange` is defined without explicitly setting this attribute to "true". See the :ref:`Activation clamping ` - section for more details. + runtime. If false, activation clamping is disabled. If "auto" and :at:`autolimits` is set in + :ref:`compiler `, activation clamping will automatically be set to true if :at:`actrange` is defined + without explicitly setting this attribute to "true". See the :ref:`Activation clamping ` section for more + details. :at:`ctrlrange`: :at-val:`real(2), "0 0"` Range for clamping the control input. The compiler expects the first value to be smaller than the second value. + |br| Setting this attribute without specifying :at:`ctrllimited` is an error, unless :at:`autolimits` is set in + :ref:`compiler `. :at:`forcerange`: :at-val:`real(2), "0 0"` Range for clamping the force output. The compiler expects the first value to be no greater than the second value. + |br| Setting this attribute without specifying :at:`forcelimited` is an error, unless :at:`autolimits` is set in + :ref:`compiler `. :at:`actrange`: :at-val:`real(2), "0 0"` Range for clamping the activation state. The compiler expects the first value to be no greater than the second value. See the :ref:`Activation clamping ` section for more details. + |br| Setting this attribute without specifying :at:`actlimited` is an error, unless :at:`autolimits` is set in + :ref:`compiler `. :at:`lengthrange`: :at-val:`real(2), "0 0"` Range of feasible lengths of the actuator's transmission. See :ref:`Length Range `. :at:`gear`: :at-val:`real(6), "1 0 0 0 0 0"` diff --git a/doc/changelog.rst b/doc/changelog.rst index 621b8af6..3b24bfdf 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -31,13 +31,18 @@ General - Cartesian 6D end-effector control is now possible by adding a reference site to actuators with :at:`site` transmission. See description of new :at:`refsite` attribute in the :ref:`actuator` documentation and `refsite.xml `_ example model. -- Joint and tendon ``limited`` attribute and actuator ``ctrllimited``, ``forcelimited`` and ``actlimited`` attributes - now default to ``auto`` rather than ``false``. Limits are automatically set to ``true`` if the corresponding range *is - defined* and ``false`` otherwise. + +- Added :at:`autolimits` compiler option. If ``true``, joint and tendon :at:`limited` attributes and actuator + :at:`ctrllimited`, :at:`forcelimited` and :at:`actlimited` attributes will automatically be set to ``true`` if the + corresponding range *is defined* and ``false`` otherwise. + + If ``autolimits="false"`` (the default) models where a :at:`range` attribute is specified without the :at:`limited` + attribute will fail to compile. A future release will change the default of :at:`autolimits` to ``true``, and this + compilation error allows users to catch this future change of behavior. .. attention:: - This is a minor breaking change. In models where a range was defined but :at:`limited` was unspecified, the target - element will now be limited. Explicitly set limited to ``false`` to revert to the previous behavior. + This is a breaking change. In models where a range was defined but :at:`limited` was unspecified, explicitly set + limited to ``false`` or remove the range to maintain the current behavior of your model. - Added moment of inertia computation for all well-formed meshes. This option is activated by setting the compiler flag :at:`exactmeshinertia` to ``true`` (defaults to ``false``). This default may change in the future. diff --git a/src/user/user_model.cc b/src/user/user_model.cc index 8ab11731..e2db338a 100644 --- a/src/user/user_model.cc +++ b/src/user/user_model.cc @@ -79,6 +79,8 @@ mjCModel::mjCModel() { modelfiledir.clear(); //------------------------ compiler settings + // TODO(b/245077553): Toggle to true by default. + autolimits = false; boundmass = 0; boundinertia = 0; settotalmass = -1; diff --git a/src/user/user_model.h b/src/user/user_model.h index 5f26fff4..f73cb685 100644 --- a/src/user/user_model.h +++ b/src/user/user_model.h @@ -107,6 +107,7 @@ class mjCModel { std::vector defaults; // settings for each defaults class //------------------------ compiler settings + bool autolimits; // infer "limited" attribute based on range double boundmass; // enfore minimum body mass double boundinertia; // enfore minimum body diagonal inertia double settotalmass; // rescale masses and inertias; <=0: ignore diff --git a/src/user/user_objects.cc b/src/user/user_objects.cc index 6ca810d9..69880e61 100644 --- a/src/user/user_objects.cc +++ b/src/user/user_objects.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -62,6 +63,18 @@ static void checksize(double* size, mjtGeom type, mjCBase* object, const char* n } } +// error message for missing "limited" attribute +static void checklimited( + const mjCBase* obj, + bool autolimits, const char* entity, const char* attr, int limited, bool hasrange) { + if (!autolimits && limited == 2 && hasrange) { + std::stringstream ss; + ss << entity << " has `" << attr << "range` but not `" << attr << "limited`. " + << "set the autolimits=\"true\" compiler option, specify `" << attr << "limited` " + << "explicitly (\"true\" or \"false\"), or remove the `" << attr << "range` attribute."; + throw mjCError(obj, "%s", ss.str().c_str()); + } +} //------------------------- class mjCError implementation ------------------------------------------ @@ -857,7 +870,9 @@ int mjCJoint::Compile(void) { } // otherwise if limited is auto, set according to whether range is specified else if (limited==2) { - limited = (range[0]==0 && range[1]==0) ? 0 : 1; + bool hasrange = !(range[0]==0 && range[1]==0); + checklimited(this, model->autolimits, "joint", "", limited, hasrange); + limited = hasrange ? 1 : 0; } // resolve limits @@ -3222,7 +3237,9 @@ void mjCTendon::Compile(void) { // if limited is auto, set to 1 if range is specified, otherwise unlimited if (limited==2) { - limited = (range[0]==0 && range[1]==0) ? 0 : 1; + bool hasrange = !(range[0]==0 && range[1]==0); + checklimited(this, model->autolimits, "tendon", "", limited, hasrange); + limited = hasrange ? 1 : 0; } // check limits @@ -3389,13 +3406,19 @@ void mjCActuator::Compile(void) { // if limited is auto, set to 1 if range is specified, otherwise unlimited if (forcelimited==2) { - forcelimited = (forcerange[0]==0 && forcerange[1]==0) ? 0 : 1; + bool hasrange = !(forcerange[0]==0 && forcerange[1]==0); + checklimited(this, model->autolimits, "actuator", "force", forcelimited, hasrange); + forcelimited = hasrange ? 1 : 0; } if (ctrllimited==2) { - ctrllimited = (ctrlrange[0]==0 && ctrlrange[1]==0) ? 0 : 1; + bool hasrange = !(ctrlrange[0]==0 && ctrlrange[1]==0); + checklimited(this, model->autolimits, "actuator", "ctrl", ctrllimited, hasrange); + ctrllimited = hasrange ? 1 : 0; } if (actlimited==2) { - actlimited = (actrange[0]==0 && actrange[1]==0) ? 0 : 1; + bool hasrange = !(actrange[0]==0 && actrange[1]==0); + checklimited(this, model->autolimits, "actuator", "act", actlimited, hasrange); + actlimited = hasrange ? 1 : 0; } // check limits diff --git a/src/xml/xml_native_reader.cc b/src/xml/xml_native_reader.cc index e3f35797..20665c04 100644 --- a/src/xml/xml_native_reader.cc +++ b/src/xml/xml_native_reader.cc @@ -46,8 +46,8 @@ static const int nMJCF = 165; static const char* MJCF[nMJCF][mjXATTRNUM] = { {"mujoco", "!", "1", "model"}, {"<"}, - {"compiler", "*", "18", "boundmass", "boundinertia", "settotalmass", "balanceinertia", - "strippath", "coordinate", "angle", "fitaabb", "eulerseq", + {"compiler", "*", "19", "autolimits", "boundmass", "boundinertia", "settotalmass", + "balanceinertia", "strippath", "coordinate", "angle", "fitaabb", "eulerseq", "meshdir", "texturedir", "discardvisual", "convexhull", "usethread", "fusestatic", "inertiafromgeom", "inertiagrouprange", "exactmeshinertia"}, {"<"}, @@ -765,6 +765,9 @@ void mjXReader::Compiler(XMLElement* section, mjCModel* mod) { int n; // top-level attributes + if (MapValue(section, "autolimits", &n, bool_map, 2)) { + mod->autolimits = (n==1); + } ReadAttr(section, "boundmass", 1, &mod->boundmass, text); ReadAttr(section, "boundinertia", 1, &mod->boundinertia, text); ReadAttr(section, "settotalmass", 1, &mod->settotalmass, text); diff --git a/src/xml/xml_native_writer.cc b/src/xml/xml_native_writer.cc index a3b8af5a..0d4f8a2e 100644 --- a/src/xml/xml_native_writer.cc +++ b/src/xml/xml_native_writer.cc @@ -696,6 +696,9 @@ void mjXWriter::Compiler(XMLElement* root) { if (model->exactmeshinertia) { WriteAttrTxt(section, "exactmeshinertia", "true"); } + // always enable autolimits. limited attributes will be written appropriately + // TODO(b/245077553): Remove this when the default is true. + WriteAttrTxt(section, "autolimits", "true"); } diff --git a/test/engine/engine_core_smooth_test.cc b/test/engine/engine_core_smooth_test.cc index 573f4dd0..9c14a4c1 100644 --- a/test/engine/engine_core_smooth_test.cc +++ b/test/engine/engine_core_smooth_test.cc @@ -206,6 +206,7 @@ TEST_F(CoreSmoothTest, RefsiteBringsToPose) { constexpr char kRefsitePath[] = "engine/testdata/refsite.xml"; const std::string xml_path = GetTestDataFilePath(kRefsitePath); mjModel* model = mj_loadXML(xml_path.c_str(), nullptr, 0, 0); + ASSERT_THAT(model, ::testing::NotNull()); mjData* data = mj_makeData(model); // set pose target in ctrl (3 positions, 3 rotations) diff --git a/test/engine/testdata/activation.xml b/test/engine/testdata/activation.xml index d6c670d8..2141b623 100644 --- a/test/engine/testdata/activation.xml +++ b/test/engine/testdata/activation.xml @@ -1,4 +1,5 @@ +