> -----Original Message----- > From: Burkhard Ilsen [mailto:burkhardil...@gmail.com] > Sent: Friday, April 07, 2017 3:30 PM > To: linuxptp-devel@lists.sourceforge.net > Subject: [Linuxptp-devel] [PATCH] tsproc_update_offset() fails when it should > succeed > > After the first SERVO_JUMP the following calls of > tsproc_update_offset() from clock_synchronize() fail > because t3 was reset. > But the offset calculation does not necessarily depend on t3, > i.e. when filtered_delay is available and neither raw nor weighting > is used. > > Signed-off-by: Burkhard Ilsen <burkhardil...@gmail.com> > --- > servo.c | 2 +- > tsproc.c | 11 ++++++++--- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/servo.c b/servo.c > index 8be4b92..809e65e 100644 > --- a/servo.c > +++ b/servo.c > @@ -53,7 +53,7 @@ struct servo *servo_create(struct config *cfg, enum > servo_type type, > } > > if (!servo) > - return NULL; > + return NULL; > > servo_step_threshold = config_get_double(cfg, NULL, > "step_threshold"); > if (servo_step_threshold > 0.0) { > diff --git a/tsproc.c b/tsproc.c > index cf5f0dc..a9c1c48 100644 > --- a/tsproc.c > +++ b/tsproc.c > @@ -163,12 +163,17 @@ int tsproc_update_offset(struct tsproc *tsp, tmv_t > *offset, double *weight) > { > tmv_t delay, raw_delay = 0; > > - if (tmv_is_zero(tsp->t1) || tmv_is_zero(tsp->t2) || > - tmv_is_zero(tsp->t3)) > + if (tmv_is_zero(tsp->t1) || tmv_is_zero(tsp->t2)) > return -1; > > - if (tsp->raw_mode || tsp->weighting) > + if (!tsp->raw_mode && tmv_is_zero(tsp->filtered_delay)) > + return -1; > +
So t1, t2, t3, and t4 usually can't be zero, so using zero as an "empty" value makes sense here. However, as Miroslav said, filtered_delay could theoretically be zero, so we might either not need a value or we need to use some other method for determining if it's valid. A flag could work? Or maybe we don't even need this check at all? It wasn't there in the original code. What is the goal of this check, what are we trying to prevent here? Just ensure that we don't use the initial filterd_delay until after we've calculated it at least once? Thanks, Jake > + if (tsp->raw_mode || tsp->weighting) { > + if (tmv_is_zero(tsp->t3)) > + return -1; > raw_delay = get_raw_delay(tsp); > + } > > delay = tsp->raw_mode ? raw_delay : tsp->filtered_delay; > > -- > 2.12.2.windows.1 > > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > Linuxptp-devel mailing list > Linuxptp-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel