Merge pull request #299 from ljbade/improve-calc-sat-state-error

Improve calc_sat_state error checking

Author: ljbade

DEVELOPMENT.rst DEVELOPMENT.mdDEVELOPMENT.rst renamed to DEVELOPMENT.md

@@ -12,7 +12,7 @@ Tools needed:
 
 To get started, run::
 
-  ./checks/setup-hooks.sh [DIR]
+    ./checks/setup-hooks.sh [DIR]
 
 from within the libswiftnav root directory. The default build-dir will be `build`.
 
@@ -22,6 +22,23 @@ in this build directory. These checks require doxygen, gcovr, and diff-cover
 
 To manually run the coverage task, use `make check-coverage`. For syntax, use `make check-style`.
 
+Building/Testing Python
+=======================
+
+To build and test the python bindings use these commands:
+
+First build and locally install libswiftnav:
+
+    cd build
+    make
+    make DESTDIR="./install" install
+
+Then build the python bindings:
+
+    cd ../python
+    export LD_LIBRARY_PATH=../build/install/usr/local/lib
+    tox
+
 Issues
 ======
 
@@ -31,7 +48,7 @@ You may see this in the output from diff-cover.
 
 Did you set up your build with::
 
-  cmake -DCMAKE_BUILD_TYPE=Coverage ..
+    cmake -DCMAKE_BUILD_TYPE=Coverage ..
 
 or with the :code:`setup-hooks.sh` script? The :code:`Coverage` flag is
 required.  If you are manually running the coverage script, make sure you run

README.md

@@ -17,7 +17,7 @@ For Doxygen documentation, see [docs.swift-nav.com.](http://docs.swift-nav.com/l
 
 For installation, see [docs/install.dox.](http://docs.swift-nav.com/libswiftnav/install.html)
 
-For development help, see [DEVELOPMENT.rst](https://github.com/swift-nav/libswiftnav/blob/master/DEVELOPMENT.rst)
+For development help, see [DEVELOPMENT.md](https://github.com/swift-nav/libswiftnav/blob/master/DEVELOPMENT.md)
 
 [1]: https://travis-ci.org/swift-nav/libswiftnav.svg?branch=master
 [2]: https://travis-ci.org/swift-nav/libswiftnav

python/tests/test_ephemeris.py

@@ -54,8 +54,10 @@ def test_sat_state():
   assert clock_err - 0.00336435649383 < tol
   assert clock_rate_err - 3.63797880709e-12 < tol
   assert not eph.is_valid(t.GpsTime(**{ 'wn': 1866, 'tow': 518400.0,}))
-  assert not eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*4}))
-  assert eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*2}))
+  assert not eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*2 + 1}))
+  assert not eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 - 3600.*2 - 1}))
+  assert eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*1}))
+  assert eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 - 3600.*1}))
   assert not eph.is_valid(t.GpsTime(**{ 'wn': 1868, 'tow': 518400.0,}))
   assert len(eph.to_dict()) == 8
   assert repr(eph)
@@ -150,6 +152,6 @@ def test_sat_state():
                                     'af0': 0.0004458986222743988,
                                     'af1': 3.637978807091713e-12,
                                     'af2': 0.0}})
-  assert eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*4}))
-  assert not eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*6}))
+  assert eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*2}))
+  assert not eph.is_valid(t.GpsTime(**{ 'wn': 1867, 'tow': 518400.0 + 3600.*3 + 1}))
   assert eph.is_healthy()

src/bits.c

@@ -124,7 +124,7 @@ void setbits(u8 *buff, u32 pos, u32 len, s32 data)
  * The method performs in-place shift operation.
  *
  * \param[in,out] buf   Pointer to buffer head.
- * \param[in]     bits  Number of bytes in the buffer.
+ * \param[in]     size  Number of bytes in the buffer.
  * \param[in]     shift Number of bits for left shift operation.
  *
  * \return None
@@ -133,7 +133,7 @@ void bitshl(void *buf, u32 size, u32 shift)
 {
   if (shift > size * CHAR_BIT) {
     /* Quick check: if the shift is larger, than the buffer, zero the data */
-    memset(buf, size, 0);
+    memset(buf, 0, size);
     return;
   }
 

src/ephemeris.c

@@ -41,15 +41,6 @@ static s8 calc_sat_state_xyz(const ephemeris_t *e, const gps_time_t *t,
                              double pos[3], double vel[3],
                              double *clock_err, double *clock_rate_err)
 {
-  /*
-   *Ephemeris did not get time-stammped when it was received.
-   */
-  if (e->toe.wn == 0)
-    return -1;
-
-  if (!e->valid || !e->healthy)
-    return -1;
-
   const ephemeris_xyz_t *ex = &e->xyz;
   u8 ndays = t->tow / DAY_SECS;
   double tod = t->tow - DAY_SECS * ndays;
@@ -111,12 +102,6 @@ static s8 calc_sat_state_kepler(const ephemeris_t *e,
   /* Seconds from the time from ephemeris reference epoch (toe) */
   dt = gpsdifftime(t, &e->toe);
 
-  /* If dt is greater than fit_interval hours our ephemeris isn't valid. */
-  if (fabs(dt) > ((u32)e->fit_interval)*60*60) {
-    log_error("Using ephemeris outside validity period, dt = %+.0f", dt);
-    return -1;
-  }
-
   /* Calculate position per IS-GPS-200D p 97 Table 20-IV */
 
   /* Semi-major axis in meters. */
@@ -217,7 +202,7 @@ static s8 calc_sat_state_kepler(const ephemeris_t *e,
  *                       clock error [s/s]
  *
  * \return  0 on success,
- *         -1 if ephemeris is older (or newer) than 4 hours
+ *         -1 if ephemeris is invalid
  */
 s8 calc_sat_state(const ephemeris_t *e, const gps_time_t *t,
                   double pos[3], double vel[3],
@@ -228,15 +213,21 @@ s8 calc_sat_state(const ephemeris_t *e, const gps_time_t *t,
   assert(clock_err != NULL);
   assert(clock_rate_err != NULL);
   assert(e != NULL);
-  assert(ephemeris_valid(e, t));
+
+  if (!ephemeris_valid(e, t)) {
+    char buf[SID_STR_LEN_MAX];
+    sid_to_string(buf, sizeof(buf), e->sid);
+    log_error("Using invalid or too old ephemeris in calc_sat_state for %s", buf);
+    return -1;
+  }
 
   switch (e->sid.constellation) {
   case CONSTELLATION_GPS:
     return calc_sat_state_kepler(e, t, pos, vel, clock_err, clock_rate_err);
   case CONSTELLATION_SBAS:
     return calc_sat_state_xyz(e, t, pos, vel, clock_err, clock_rate_err);
   default:
-    assert("unsupported constellation");
+    assert("Unsupported constellation");
     return -1;
   }
 }
@@ -250,12 +241,36 @@ s8 calc_sat_state(const ephemeris_t *e, const gps_time_t *t,
  */
 u8 ephemeris_valid(const ephemeris_t *eph, const gps_time_t *t)
 {
+  assert(eph != NULL);
+  assert(t != NULL);
+
+  if (!eph->valid) {
+    return 0;
+  }
+
+  if (eph->fit_interval <= 0) {
+    log_warn("ephemeris_valid used with 0 eph->fit_interval");
+    return 0;
+  }
+
+  /*
+   *Ephemeris did not get time-stammped when it was received.
+   */
+  if (eph->toe.wn == 0) {
+    return 0;
+  }
+
   /* Seconds from the time from ephemeris reference epoch (toe) */
   double dt = gpsdifftime(t, &eph->toe);
 
   /* TODO: this doesn't exclude ephemerides older than a week so could be made
    * better. */
-  return (eph->valid && fabs(dt) < ((u32)eph->fit_interval)*60*60);
+  /* If dt is greater than fit_interval / 2 hours our ephemeris isn't valid. */
+  if (fabs(dt) > ((u32)eph->fit_interval / 2) * 60 * 60) {
+    return 0;
+  }
+
+  return 1;
 }
 
 /** Is this satellite healthy? Note this function only checks flags in the