> -----Original Message-----
> From: Burkhard Ilsen [mailto:[email protected]]
> Sent: Friday, April 07, 2017 3:30 PM
> To: [email protected]
> 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 <[email protected]>
> ---
> 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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
