From 2d1c843bfbbc6df25e0f66e736a96739416f36e2 Mon Sep 17 00:00:00 2001 From: Daniel_M_Williams Date: Tue, 28 Oct 2014 10:25:42 -0600 Subject: [PATCH 1/4] [Fix] Fixed bug in RSE file loading ( XML-formatted engine description) This patch is based off a thread from "The Rocketry Forum" ( http://www.rocketryforum.com/showthread.php?70475-Problems-with-custom-motors-in-OpenRocket ) . Complaint: Poster had a water-rocket engine file which would raise IOExceptions upon automatic load. (1) the original .rse motor file, posted in the thread above, has an invalid motor length. (2) The corrected .rse file follows at the end of this commit message. Bug: The mass auto-scaling code would over-estimate the mass loss, and declare a negative mass upon motor exhaustion. The negative mass is small (in this case, ~= -1E-16). However, this error is larger than the single-precision epsilon. Fix: Added a loop which checked for negative mass during the thrust curve, and reset those values back to zero. (AbstractMotorLoader.java:105) Also, expanded the number and detail level of some related exception reporting. (ThrustCurveMotor:137) ---- Water Rocket, 90 psi, 800g --- .../file/motor/AbstractMotorLoader.java | 20 +++++++++++-------- .../file/motor/RockSimMotorLoader.java | 7 ++++--- .../sf/openrocket/motor/ThrustCurveMotor.java | 9 ++++++--- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/core/src/net/sf/openrocket/file/motor/AbstractMotorLoader.java b/core/src/net/sf/openrocket/file/motor/AbstractMotorLoader.java index 365a47d53..b12c0306a 100644 --- a/core/src/net/sf/openrocket/file/motor/AbstractMotorLoader.java +++ b/core/src/net/sf/openrocket/file/motor/AbstractMotorLoader.java @@ -15,7 +15,7 @@ import net.sf.openrocket.util.MathUtil; public abstract class AbstractMotorLoader implements MotorLoader { - + /** * {@inheritDoc} *

@@ -40,7 +40,6 @@ public abstract class AbstractMotorLoader implements MotorLoader { protected abstract List load(Reader reader, String filename) throws IOException; - /** * Return the default charset to use when loading rocket files of this type. *

@@ -52,11 +51,11 @@ public abstract class AbstractMotorLoader implements MotorLoader { protected abstract Charset getDefaultCharset(); - - + + ////////// Helper methods ////////// - + /** * Calculate the mass of a motor at distinct points in time based on the * initial total mass, propellant weight and thrust. @@ -88,6 +87,7 @@ public abstract class AbstractMotorLoader implements MotorLoader { double f1 = thrust.get(i); double dm = 0.5 * (f0 + f1) * (t1 - t0); + deltam.add(dm); totalMassChange += dm; t0 = t1; @@ -102,10 +102,15 @@ public abstract class AbstractMotorLoader implements MotorLoader { mass.add(total); } + // to correct negative mass error condition: (caused by rounding errors in the above loops) + for (int mass_index = 0; mass_index < mass.size(); mass_index++) { + if (mass.get(mass_index) < 0) { + mass.set(mass_index, 0.0d); + } + } return mass; } - /** * Helper method to remove a delay (or plugged) from the end of a motor designation, * if present. @@ -121,7 +126,6 @@ public abstract class AbstractMotorLoader implements MotorLoader { } - /** * Helper method to tokenize a string using whitespace as the delimiter. */ @@ -168,7 +172,7 @@ public abstract class AbstractMotorLoader implements MotorLoader { } - + @SuppressWarnings("unchecked") protected static void finalizeThrustCurve(List time, List thrust, List... lists) { diff --git a/core/src/net/sf/openrocket/file/motor/RockSimMotorLoader.java b/core/src/net/sf/openrocket/file/motor/RockSimMotorLoader.java index ec1bd3156..838e2d8fa 100644 --- a/core/src/net/sf/openrocket/file/motor/RockSimMotorLoader.java +++ b/core/src/net/sf/openrocket/file/motor/RockSimMotorLoader.java @@ -135,8 +135,8 @@ public class RockSimMotorLoader extends AbstractMotorLoader { private final double initMass; private final double propMass; private final Motor.Type type; - private boolean calculateMass; - private boolean calculateCG; + private boolean calculateMass = false; + private boolean calculateCG = false; private String description = ""; @@ -324,8 +324,8 @@ public class RockSimMotorLoader extends AbstractMotorLoader { if (time == null || time.size() == 0) throw new SAXException("Illegal motor data"); - finalizeThrustCurve(time, force, mass, cg); + final int n = time.size(); if (hasIllegalValue(mass)) @@ -336,6 +336,7 @@ public class RockSimMotorLoader extends AbstractMotorLoader { if (calculateMass) { mass = calculateMass(time, force, initMass, propMass); } + if (calculateCG) { for (int i = 0; i < n; i++) { cg.set(i, length / 2); diff --git a/core/src/net/sf/openrocket/motor/ThrustCurveMotor.java b/core/src/net/sf/openrocket/motor/ThrustCurveMotor.java index 3a0e71a2a..888882af9 100644 --- a/core/src/net/sf/openrocket/motor/ThrustCurveMotor.java +++ b/core/src/net/sf/openrocket/motor/ThrustCurveMotor.java @@ -134,11 +134,14 @@ public class ThrustCurveMotor implements Motor, Comparable, Se if (c.isNaN()) { throw new IllegalArgumentException("Invalid CG " + c); } - if (c.x < 0 || c.x > length) { - throw new IllegalArgumentException("Invalid CG position " + c.x); + if (c.x < 0) { + throw new IllegalArgumentException("Invalid CG position " + String.format("%f", c.x) + ": CG is below the start of the motor."); + } + if (c.x > length) { + throw new IllegalArgumentException("Invalid CG position: " + String.format("%f", c.x) + ": CG is above the end of the motor."); } if (c.weight < 0) { - throw new IllegalArgumentException("Negative mass " + c.weight); + throw new IllegalArgumentException("Negative mass " + c.weight + "at time=" + time[Arrays.asList(cg).indexOf(c)]); } } From ab5d8f2a51507a7b568c3889e3f22ad179e9a0eb Mon Sep 17 00:00:00 2001 From: Daniel_M_Williams Date: Tue, 28 Oct 2014 13:37:38 -0600 Subject: [PATCH 2/4] [Fix][Cont] Added Unit test for RSE-load negative mass bug. - Added Test case in TestMotorLoader.java - Added Test load file 'test3.rse' which would previously raise an IOException on load. - Added this test case to the 'testGeneralMotorLoader' method as well. - Adjusted the digest string so that 'test3.rse' will pass the unit tests. - TestMotorLoader currently passes 5/5 unit tests. --- .../file/motor/TestMotorLoader.java | 12 +++++++-- .../net/sf/openrocket/file/motor/test3.rse | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 core/test/net/sf/openrocket/file/motor/test3.rse diff --git a/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java b/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java index dbc23e745..ee8e47978 100644 --- a/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java +++ b/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java @@ -1,6 +1,8 @@ package net.sf.openrocket.file.motor; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.io.InputStream; @@ -16,6 +18,7 @@ public class TestMotorLoader { private static final String DIGEST1 = "e523030bc96d5e63313b5723aaea267d"; private static final String DIGEST2 = "6a41f0f10b7283793eb0e6b389753729"; + private static final String DIGEST3 = "e3164a735f9a50500f2725f0a33d246b"; @Test @@ -25,7 +28,7 @@ public class TestMotorLoader { test(loader, "test1.eng", DIGEST1); test(loader, "test2.rse", DIGEST2); test(loader, "test.zip", DIGEST2, DIGEST1); - + test(loader, "test3.rse", DIGEST3); } @Test @@ -38,6 +41,11 @@ public class TestMotorLoader { test(new RockSimMotorLoader(), "test2.rse", DIGEST2); } + @Test + public void testRocksimMotorLoader3() throws IOException { + test(new RockSimMotorLoader(), "test3.rse", DIGEST3); + } + @Test public void testZipMotorLoader() throws IOException { test(new ZipFileMotorLoader(), "test.zip", DIGEST2, DIGEST1); diff --git a/core/test/net/sf/openrocket/file/motor/test3.rse b/core/test/net/sf/openrocket/file/motor/test3.rse new file mode 100644 index 000000000..95a86c555 --- /dev/null +++ b/core/test/net/sf/openrocket/file/motor/test3.rse @@ -0,0 +1,27 @@ + + + + Water Rocket, 90 psi, 800g + + + + + + + + + + + + + + + + + + From 67e741a2dfd111ab7d51f8b0d0ff14bb6c02242a Mon Sep 17 00:00:00 2001 From: Daniel_M_Williams Date: Wed, 29 Oct 2014 14:55:32 -0600 Subject: [PATCH 3/4] [Fix][Cont] Added Unit test for RSE-load negative mass bug. - Added Test case in TestMotorLoader.java - Added Test load file 'test3.rse' which would previously raise an IOException on load. - Added this test case to the 'testGeneralMotorLoader' method as well. - Adjusted the digest string so that 'test3.rse' will pass the unit tests. - TestMotorLoader currently passes 5/5 unit tests. --- .../file/motor/TestMotorLoader.java | 12 +++++++-- .../net/sf/openrocket/file/motor/test3.rse | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 core/test/net/sf/openrocket/file/motor/test3.rse diff --git a/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java b/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java index dbc23e745..ee8e47978 100644 --- a/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java +++ b/core/test/net/sf/openrocket/file/motor/TestMotorLoader.java @@ -1,6 +1,8 @@ package net.sf.openrocket.file.motor; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.io.InputStream; @@ -16,6 +18,7 @@ public class TestMotorLoader { private static final String DIGEST1 = "e523030bc96d5e63313b5723aaea267d"; private static final String DIGEST2 = "6a41f0f10b7283793eb0e6b389753729"; + private static final String DIGEST3 = "e3164a735f9a50500f2725f0a33d246b"; @Test @@ -25,7 +28,7 @@ public class TestMotorLoader { test(loader, "test1.eng", DIGEST1); test(loader, "test2.rse", DIGEST2); test(loader, "test.zip", DIGEST2, DIGEST1); - + test(loader, "test3.rse", DIGEST3); } @Test @@ -38,6 +41,11 @@ public class TestMotorLoader { test(new RockSimMotorLoader(), "test2.rse", DIGEST2); } + @Test + public void testRocksimMotorLoader3() throws IOException { + test(new RockSimMotorLoader(), "test3.rse", DIGEST3); + } + @Test public void testZipMotorLoader() throws IOException { test(new ZipFileMotorLoader(), "test.zip", DIGEST2, DIGEST1); diff --git a/core/test/net/sf/openrocket/file/motor/test3.rse b/core/test/net/sf/openrocket/file/motor/test3.rse new file mode 100644 index 000000000..95a86c555 --- /dev/null +++ b/core/test/net/sf/openrocket/file/motor/test3.rse @@ -0,0 +1,27 @@ + + + + Water Rocket, 90 psi, 800g + + + + + + + + + + + + + + + + + + From d905d24f478747cc2b4ade75a8186e91e785020d Mon Sep 17 00:00:00 2001 From: Daniel_M_Williams Date: Wed, 29 Oct 2014 14:59:25 -0600 Subject: [PATCH 4/4] re-apply patch 5c0d9eb... which was clobbered by mistake --- core/src/net/sf/openrocket/utils/L10nPropertyReport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/net/sf/openrocket/utils/L10nPropertyReport.java b/core/src/net/sf/openrocket/utils/L10nPropertyReport.java index d7001977a..0241678a9 100644 --- a/core/src/net/sf/openrocket/utils/L10nPropertyReport.java +++ b/core/src/net/sf/openrocket/utils/L10nPropertyReport.java @@ -10,7 +10,7 @@ import java.util.Properties; public class L10nPropertyReport { - private static String[] supportedLocales = new String[] { "en", "de", "es", "fr", "it", "ru", "cs", "pl", "ja", "pt", "uk_UA" }; + private static String[] supportedLocales = new String[] { "en", "de", "es", "fr", "it", "ru", "cs", "pl", "ja", "pt", "tr", "zh_CN", "uk_UA" }; public static void main(String[] args) throws Exception {