> -----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

Reply via email to