Changes to dcmotor:

- Remove `lugre:viscous`, should now be added directly to actuator `damping`. Trying to do this for the user was incompatible with default inheritance (compounding instead of overriding).
- Move voltage limiting from the `saturation` to the `controller` attribute.
- Fix indexing issues in default inheritance.

PiperOrigin-RevId: 897087642
Change-Id: I5388c2633e15c7e223992e7eb5d6a28db75a6438

test/engine/engine_forward_test.cc

@@ -1416,7 +1416,7 @@ TEST_F(DCMotorTest, LuGreViscousFriction) {
     </worldbody>
     <actuator>
       <dcmotor joint="joint" motorconst="0.05" resistance="2.0"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               damping="0.01" lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";
@@ -1929,7 +1929,7 @@ TEST_F(DCMotorTest, CurrentRateLimit) {
     </worldbody>
     <actuator>
       <dcmotor joint="joint" motorconst="0.05" resistance="2.0"
-               inductance="0.01 0" saturation="0 0 100 0"/>
+               inductance="0.01 0" saturation="0 0 100"/>
     </actuator>
   </mujoco>
   )";
@@ -1963,6 +1963,106 @@ TEST_F(DCMotorTest, CurrentRateLimit) {
   mj_deleteModel(model);
 }
 
+
+TEST_F(DCMotorTest, VoltageLimit) {
+  // verifies that saturation:voltage clamps voltage
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <worldbody>
+      <body>
+        <joint name="joint"/>
+        <geom size="1"/>
+      </body>
+    </worldbody>
+    <actuator>
+      <dcmotor joint="joint" motorconst="0.05" resistance="2.0"
+               input="position" controller="1 0 0 0 0 10.0"/>
+    </actuator>
+  </mujoco>
+  )";
+  char error[1024];
+  mjModel* model = LoadModelFromString(xml, error, sizeof(error));
+  ASSERT_THAT(model, NotNull()) << error;
+  mjData* data = mj_makeData(model);
+
+  // Vmax = 10.0, ctrl = 20.0
+  // force = K/R * Vmax = 0.05 / 2.0 * 10.0 = 0.25
+  data->ctrl[0] = 20.0;
+  mj_forward(model, data);
+
+  EXPECT_NEAR(data->actuator_force[0], 0.25, MjTol(1e-12, 1e-5));
+
+  // negative drive
+  data->ctrl[0] = -20.0;
+  mj_forward(model, data);
+
+  EXPECT_NEAR(data->actuator_force[0], -0.25, MjTol(1e-12, 1e-5));
+
+  mj_deleteData(data);
+  mj_deleteModel(model);
+}
+
+
+TEST_F(DCMotorTest, IntegralClamp) {
+  // verifies that controller Imax clamps integral state
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <option timestep="0.001"/>
+    <worldbody>
+      <body>
+        <joint name="joint"/>
+        <geom size="1"/>
+      </body>
+    </worldbody>
+    <actuator>
+      <dcmotor joint="joint" input="position" controller="2.0 0.5 0 0 5.0"
+               motorconst="0.05" resistance="2.0"/>
+    </actuator>
+  </mujoco>
+  )";
+  char error[1024];
+  mjModel* model = LoadModelFromString(xml, error, sizeof(error));
+  ASSERT_THAT(model, NotNull()) << error;
+  mjData* data = mj_makeData(model);
+
+  // Imax = 5.0
+  ASSERT_EQ(model->actuator_actnum[0], 1);  // only ki is stateful
+  int adr = model->actuator_actadr[0];
+
+  // set integral state to Imax
+  data->act[adr] = 5.0;
+
+  // set target to generate positive error (ctrl - length)
+  data->ctrl[0] = 1.0;  // target
+  data->qpos[0] = 0.0;  // length = 0
+
+  mj_forward(model, data);
+
+  // act_dot should be clamped to 0 because act >= Imax and error > 0
+  EXPECT_NEAR(data->act_dot[adr], 0.0, MjTol(1e-12, 1e-5));
+
+  // set target to generate negative error
+  data->ctrl[0] = -1.0;
+  mj_forward(model, data);
+
+  // act_dot should be negative (not clamped)
+  EXPECT_NEAR(data->act_dot[adr], -1.0, MjTol(1e-12, 1e-5));
+
+  // set integral state to -Imax
+  data->act[adr] = -5.0;
+
+  // set target to generate negative error
+  data->ctrl[0] = -1.0;
+  data->qpos[0] = 0.0;
+  mj_forward(model, data);
+
+  // act_dot should be clamped to 0 because act <= -Imax and error < 0
+  EXPECT_NEAR(data->act_dot[adr], 0.0, MjTol(1e-12, 1e-5));
+
+  mj_deleteData(data);
+  mj_deleteModel(model);
+}
+
 TEST_F(DCMotorTest, LuGreExactIntegration) {
   static constexpr char xml[] = R"(
   <mujoco>
@@ -1975,7 +2075,7 @@ TEST_F(DCMotorTest, LuGreExactIntegration) {
     </worldbody>
     <actuator>
       <dcmotor joint="joint" motorconst="0.05" resistance="2.0"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               damping="0.01" lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";
@@ -2021,7 +2121,7 @@ TEST_F(DCMotorTest, LuGreSteadyState) {
     </worldbody>
     <actuator>
       <dcmotor joint="joint" motorconst="0.05" resistance="2.0"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               damping="0.01" lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";
@@ -2068,7 +2168,7 @@ TEST_F(DCMotorTest, LuGreBristleSpring) {
     </worldbody>
     <actuator>
       <dcmotor joint="joint" motorconst="0.05" resistance="2.0"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               damping="0.01" lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";

test/engine/testdata/derivative/dcmotor.xml

@@ -30,6 +30,6 @@
 
     <!-- DC motor with LuGre friction (sigma1 micro-damping) -->
     <dcmotor name="dc_lugre" joint="joint4" motorconst="0.05" resistance="2.0"
-             lugre="1e4 100 0.001 0.005 0.008 0.1"/>
+             damping="0.001" lugre="1e4 100 0.005 0.008 0.1"/>
   </actuator>
 </mujoco>

test/user/user_api_test.cc

@@ -280,6 +280,55 @@ TEST_F(MujocoTest, SetToDCMotorDeriveKe) {
   mj_deleteSpec(spec);
 }
 
+TEST_F(MujocoTest, SetToDCMotorFull) {
+  mjSpec* spec = mj_makeSpec();
+  mjsActuator* actuator = mjs_addActuator(spec, 0);
+
+  double motorconst[2] = {0.05, 0.05};
+  double resistance = 2.0;
+  double saturation[3] = {1.0, 2.0, 3.0};
+  double controller[6] = {10.0, 20.0, 30.0, 40.0, 50.0, 60.0};
+
+  const char* err = mjs_setToDCMotor(actuator, motorconst, resistance,
+                                     nullptr, saturation, nullptr,
+                                     nullptr, controller, nullptr,
+                                     nullptr, 0);
+  EXPECT_STREQ(err, "");
+  EXPECT_EQ(actuator->gainprm[0], 2.0);   // resistance
+  EXPECT_EQ(actuator->gainprm[1], 0.05);  // K
+  EXPECT_EQ(actuator->gainprm[4], 10.0);  // kp
+  EXPECT_EQ(actuator->gainprm[5], 20.0);  // ki
+  EXPECT_EQ(actuator->gainprm[6], 30.0);  // kd
+  EXPECT_EQ(actuator->dynprm[7], 40.0);   // slewmax
+  EXPECT_EQ(actuator->dynprm[8], 50.0);   // Imax
+  EXPECT_EQ(actuator->gainprm[7], 60.0);  // Vmax
+  EXPECT_EQ(actuator->dynprm[1], 3.0);    // (di/dt)_max
+
+  mj_deleteSpec(spec);
+}
+
+TEST_F(MujocoTest, SetToDCMotorLuGre) {
+  mjSpec* spec = mj_makeSpec();
+  mjsActuator* actuator = mjs_addActuator(spec, 0);
+
+  double motorconst[2] = {0.05, 0.05};
+  double resistance = 2.0;
+  double lugre[5] = {100.0, 1.0, 0.5, 0.7, 10.0};
+
+  const char* err = mjs_setToDCMotor(actuator, motorconst, resistance,
+                                     nullptr, nullptr, nullptr,
+                                     nullptr, nullptr, nullptr,
+                                     lugre, 0);
+  EXPECT_STREQ(err, "");
+  EXPECT_EQ(actuator->dynprm[5], 100.0);  // stiffness
+  EXPECT_EQ(actuator->dynprm[6], 1.0);    // damping
+  EXPECT_EQ(actuator->biasprm[3], 0.5);   // coulomb
+  EXPECT_EQ(actuator->biasprm[4], 0.7);   // static
+  EXPECT_EQ(actuator->biasprm[5], 10.0);  // stribeck
+
+  mj_deleteSpec(spec);
+}
+
 static constexpr char xml_plugin_1[] = R"(
   <mujoco model="MuJoCo Model">
     <worldbody>

test/xml/xml_native_reader_test.cc

@@ -3108,7 +3108,55 @@ TEST_F(ActuatorParseTest, DCMotorSaturation) {
   mj_deleteModel(model);
 }
 
-TEST_F(ActuatorParseTest, DCMotorLuGreRemapping) {
+
+TEST_F(ActuatorParseTest, DCMotorInheritedDefaults) {
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <default>
+      <dcmotor motorconst="1.0" resistance="1.0" controller="2.0 0.5 0.1 10.0 5.0 12.0"
+      saturation="0 0 0" inductance="0 0.01" input="velocity"/>
+    </default>
+    <worldbody>
+      <body>
+        <geom size="1"/>
+        <joint name="jnt"/>
+      </body>
+    </worldbody>
+    <actuator>
+      <dcmotor joint="jnt" motorconst="0.05" resistance="2.0"/>
+    </actuator>
+  </mujoco>
+  )";
+  std::array<char, 1024> error;
+  mjModel* model = LoadModelFromString(xml, error.data(), error.size());
+  ASSERT_THAT(model, NotNull()) << error.data();
+
+  // check motorconst and resistance are overridden by instance
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[1], 0.05);
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[0], 2.0);
+
+  // check controller gains (kp, ki, kd) in gainprm[4:6]
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[4], 2.0);
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[5], 0.5);
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[6], 0.1);
+
+  // check controller limits (slewmax, Imax) in dynprm[7,8]
+  EXPECT_MJTNUM_EQ(model->actuator_dynprm[7], 10.0);
+  EXPECT_MJTNUM_EQ(model->actuator_dynprm[8], 5.0);
+
+  // check Vmax in gainprm[7]
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[7], 12.0);
+
+  // check input mode in gainprm[8]
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[8], 2.0);
+
+  // check inductance (te) in dynprm[0]
+  EXPECT_MJTNUM_EQ(model->actuator_dynprm[0], 0.01);
+
+  mj_deleteModel(model);
+}
+
+TEST_F(ActuatorParseTest, DCMotorControllerFull) {
   static constexpr char xml[] = R"(
   <mujoco>
     <worldbody>
@@ -3119,7 +3167,36 @@ TEST_F(ActuatorParseTest, DCMotorLuGreRemapping) {
     </worldbody>
     <actuator>
       <dcmotor joint="jnt" motorconst="0.05" resistance="2.0"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               controller="1.0 2.0 3.0 4.0 5.0 6.0"/>
+    </actuator>
+  </mujoco>
+  )";
+  std::array<char, 1024> error;
+  mjModel* model = LoadModelFromString(xml, error.data(), error.size());
+  ASSERT_THAT(model, NotNull()) << error.data();
+
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[4], 1.0);
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[5], 2.0);
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[6], 3.0);
+  EXPECT_MJTNUM_EQ(model->actuator_dynprm[7], 4.0);
+  EXPECT_MJTNUM_EQ(model->actuator_dynprm[8], 5.0);
+  EXPECT_MJTNUM_EQ(model->actuator_gainprm[7], 6.0);
+
+  mj_deleteModel(model);
+}
+
+TEST_F(ActuatorParseTest, DCMotorLuGreRemapping) {
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <worldbody>
+      <body>
+        <geom size="1"/>
+        <joint name="jnt"/>
+      </body>
+    </worldbody>
+    <actuator>
+      <dcmotor joint="jnt" motorconst="0.05" resistance="2.0" damping="0.01"
+               lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";
@@ -3135,6 +3212,30 @@ TEST_F(ActuatorParseTest, DCMotorLuGreRemapping) {
   mj_deleteModel(model);
 }
 
+TEST_F(ActuatorParseTest, DCMotorLuGreInheritedDefaults) {
+  static constexpr char xml[] = R"(
+  <mujoco>
+    <default>
+      <dcmotor motorconst="0.05" resistance="2.0" damping="0.01" lugre="100 1 0.5 0.7 10"/>
+    </default>
+    <worldbody>
+      <body>
+        <geom size="1"/>
+        <joint name="jnt"/>
+      </body>
+    </worldbody>
+    <actuator>
+      <dcmotor joint="jnt"/>
+    </actuator>
+  </mujoco>
+  )";
+  std::array<char, 1024> error;
+  mjModel* model = LoadModelFromString(xml, error.data(), error.size());
+  ASSERT_THAT(model, NotNull()) << error.data();
+  EXPECT_MJTNUM_EQ(model->actuator_damping[0], 0.01);
+  mj_deleteModel(model);
+}
+
 TEST_F(ActuatorParseTest, DCMotorActdimStateless) {
   static constexpr char xml[] = R"(
   <mujoco>
@@ -3216,7 +3317,7 @@ TEST_F(ActuatorParseTest, DCMotorActdimLuGreOnly) {
     </worldbody>
     <actuator>
       <dcmotor joint="jnt" motorconst="0.05" resistance="2.0"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";
@@ -3241,7 +3342,7 @@ TEST_F(ActuatorParseTest, DCMotorActdimAllThree) {
       <dcmotor joint="jnt" motorconst="0.05" resistance="2.0"
                inductance="0.001 0"
                thermal="10 5 0 0 0 25"
-               lugre="100 1 0.01 0.5 0.7 10"/>
+               lugre="100 1 0.5 0.7 10"/>
     </actuator>
   </mujoco>
   )";