Re: [chrony-dev] [PATCH 2/3] refclock: Include TAI offset when adding samples

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Fri, Oct 06, 2017 at 03:09:02PM -0400, Chris Perl wrote:
> @@ -76,6 +76,7 @@ struct RCL_Instance_Record {
>    int poll;
>    int leap_status;
>    int pps_forced;
> +  int tai;
>    int pps_rate;
>    int pps_active;
>    int max_lock_age;

Please move the tai field after the new stratum field.

> @@ -189,6 +190,12 @@ RCL_AddRefclock(RefclockParameters *params)
>      return 0;
>    }
>  
> +  if (params->tai && !CNF_GetLeapSecTimezone()) {
> +    LOG(LOGS_WARN,
> +       "Refclock %s configured for auto tai offset, but no leapsectz configured",
> +       UTI_RefidToString(params->ref_id));
> +  }
> +

I think this might be upgraded to a fatal error "refclock tai option
requires leapsectz" (shorter messages save space).

> @@ -385,10 +394,25 @@ RCL_AddSample(RCL_Instance instance, struct timespec *sample_time, double offset
>        return 0;
>    }
>  
> -  filter_add_sample(&instance->filter, &cooked_time, offset - correction + instance->offset, dispersion);
> +  if (instance->tai) {
> +    tai_offset = REF_GetTaiOffset();
> +  } else {
> +    tai_offset = 0;
> +  }

I'd suggest to simplify this whole hunk to something like:
  if (instance->tai)
    offset += REF_GetTaiOffset(...)

> --- a/reference.c
> +++ b/reference.c
> @@ -1355,6 +1355,17 @@ int REF_IsLeapSecondClose(void)
>  }
>  
>  /* ================================================== */
> +int REF_GetTaiOffset(void)

This function should have a "struct timespec *ts" parameter. Reference
time is independent from the local time and we want to get correct
offset even if the local time is years behind (e.g. start with no
RTC).

After the return type there should be a line break.

Also, this should probably be a separate commit. Documentation changes
don't need to be separated. One commit to add this function and
another commit to add the refclock option, including parsing and
documentation, is ok.

> +{
> +  time_t now;
> +  int tai_offset;
> +
> +  now = time(NULL);
> +  get_tz_leap(now, &tai_offset);
> +  return tai_offset;
> +}
> +

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.