Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement suggestions #7

Closed
Lefuneste83 opened this issue Sep 14, 2023 · 5 comments
Closed

Improvement suggestions #7

Lefuneste83 opened this issue Sep 14, 2023 · 5 comments

Comments

@Lefuneste83
Copy link

Lefuneste83 commented Sep 14, 2023

Hello

Thank you for your help in setting up this nice solution.

I have implemented most of your code for the LCD output of the chronyc status, and everything works as expected. I have a few remarks.

-In latest Linux distributions, in particular on Debian 12, you cannot "pip install" a package system wide anymore in standard.
You must restrict the installation of python packages to a venv. The proposed way to circumvent this is to use pipx to install packages system wise, but pipx only accepts programs pacakges, not libraries (which is the case for gpsd-py3 and smbus).
So the only possible solution is to manually delete the file "EXTERNALLY-MANAGED" located in /usr/lib/python3.xx/.
Considering that this venv constraint will be more and more widespread as systems get updated, you should consider updating your installation documentation.

-Considering the display itself, I believe it would be useful to replace some static information which takes some room, or to switch the content of a line every once in a while, in particular for one missing very important information, which is the STRATUM status of the server.
If you have a synchronization issue of any kind (like a process unable to access SHM) your server can be degraded to a higher stratum level without any notice. It will still serve time internally on your LAN, but as a lower grade reference. So I believe it would be nice to have the current stratum level directly displayed as well.

I have fiddled with the code that would allow this, but haven't been successful in doing so. It would be nice to complete this ntp_offset() function to parse the stratum level as it processes the chronyc tracking output.
You could then alternate the second line of the display every 5 seconds or so with NTP: X.XXXXXXXXXsec / Stratum level : X
Or any other useful info such as RMS offset and Frequency, both of which have a slow evolution and have some level of importance.
Sorry I am not good enough in Python to work this out myself and also it would be valuable for the community.

Thanks!

Snipet of the current function below

def ntp_offset():
    cmd = ["chronyc", "tracking"]
    ret = exec_cmd(cmd)
    for line in ret:
        pars = line.split(":", 1)
        if len(pars) == 2:
            if pars[0].strip() == "System time":
                sub_pars = pars[1].strip().split(" ")
                if len(sub_pars) > 2:
                    val = float(sub_pars[0])
                    if "slow" == sub_pars[2]:
                        val = -1.0 * val
            return "NTP: {:+12.9f}sec".format(val)
@Lefuneste83
Copy link
Author

Lefuneste83 commented Sep 14, 2023

Ok forget it, I asked my new best friend Chat GPT. It took about half a second to generate the following code which I only very slightly modified to adjust the display...
I guess coders will start to get worried...

def ntp_offset():
    cmd = ["chronyc", "tracking"]
    ret = exec_cmd(cmd)
    ntp_val = None
    stratum_val = None
    for line in ret:
        pars = line.split(":", 1)
        if len(pars) == 2:
            key = pars[0].strip()
            value = pars[1].strip()
            if key == "System time":
                sub_pars = value.split(" ")
                if len(sub_pars) > 2:
                    ntp_val = float(sub_pars[0])
                    if "slow" == sub_pars[2]:
                        ntp_val = -1.0 * ntp_val
            elif key == "Stratum":
                stratum_val = int(value)

    if ntp_val is not None and stratum_val is not None:
        return "S{} NTP:{:+12.9f}s".format(stratum_val, ntp_val)
    elif ntp_val is not None:
        return "NTP: {:+12.9f}sec".format(ntp_val)
    else:
        return "NTP and Stratum values not found"

@domschl
Copy link
Owner

domschl commented Sep 15, 2023

Thank you for the suggestions and remarks. I will look into both.

  • Having stratum information available is definitely a good idea.
  • Yes python packages became more of an effort. My bad recommendation would be to use pip with the infamous --break-system-packages flag for gpsd-py3 since it has no dependencies. venv is of course the correct solution, this would require to modify the chronotron.service file to point to the specific python instance of the venv, which requires some documentation effort. I guess that's the way to go.

I'll update this, once I've had time to look at this.

@aGGreSSiv
Copy link

Hello,

I wanted to install the screen by following the instructions, but I could not succeed in the installation due to the errors mentioned above, which I still had difficulty understanding (I am not a programmer). Have you had a chance to update the instructions to use screen?

Everything is working now, but I have to check by ssh. It would be nice to see from the screen if everything is OK.

@domschl
Copy link
Owner

domschl commented Dec 21, 2023

@aGGreSSiv : I've updated the documentation of the install process. If you are still having trouble, please open a new issue and describe at what step you're stuck, and we have a look at it!

@aGGreSSiv
Copy link

@aGGreSSiv : I've updated the documentation of the install process. If you are still having trouble, please open a new issue and describe at what step you're stuck, and we have a look at it!

Thank you very much. I will try it as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants