1748 replies to this topic

[littlesat]

Yep indeed a good idea.... 

 

I just discovered there is more going on here. Also in case with e.g. wakeup during 'deep standby' when the box has an RTC thing on board... It needs more than just a simple revision.

 

The 'only' thing I need some help with... is it possible to get a trigger when the network becomes online....

Edited by littlesat, 26 October 2023 - 17:15.

[littlesat]

                def timesyncChanged(configElement):
                                if configElement.value == "dvb" or not GetIPsFromNetworkInterfaces():
                                                eDVBLocalTimeHandler.getInstance().setUseDVBTime(True)
 
---> Sounds like here I need instead to write to enigma2 cpp code change the config config.misc.useTransponderTime.value=True instead - so that config should not become obsolete as it is used for more. It is set at the 'current status of which 'clock' us currently in use.

                                                eEPGCache.getInstance().timeUpdated()
                                                if os.path.isfile('/var/spool/cron/crontabs/root'):
                                                                Console().ePopen("sed -i '/ntpdate-sync/d' /var/spool/cron/crontabs/root;")
                                                if configElement.value == "dvb" and os.path.islink('/etc/network/if-up.d/ntpdate-sync'):
                                                                Console().ePopen("unlink /etc/network/if-up.d/ntpdate-sync")
 
-> Add here a check if the config is ntp and then poll each x secs if the network becomes online and then switch back to ntp time could be a solution. But I prefer to have a trigger for that.

                                else:
                                                eDVBLocalTimeHandler.getInstance().setUseDVBTime(False)
 
---> Sounds like here I need instead to write to enigma2 cpp code change the config config.misc.useTransponderTime.value=True instead

                                                eEPGCache.getInstance().timeUpdated()
                                                if not os.path.isfile('/var/spool/cron/crontabs/root') or not 'ntpdate-sync' in open('/var/spool/cron/crontabs/root').read():
                                                                Console().ePopen("echo '30 * * * *    /usr/bin/ntpdate-sync silent' >> /var/spool/cron/crontabs/root")
                                                if not os.path.islink('/etc/network/if-up.d/ntpdate-sync'):
                                                                Console().ePopen("ln -s /usr/bin/ntpdate-sync /etc/network/if-up.d/ntpdate-sync")
                config.ntp.timesync = ConfigSelection(default="auto", choices=[("auto", _("auto")), ("dvb", _("Transponder Time")), ("ntp", _("Internet (ntp)"))])
                config.ntp.timesync.addNotifier(timesyncChanged)
                config.ntp.server = ConfigText("pool.ntp.org", fixed_size=False)

[Dimitrij]

Stop, stop.

 

regarding the config.misc.useTransponderTime.addNotifier(useTransponderTimeChanged)... maybe this is not required anymore at all and bit became obsoleted code....

Yes, it’s true that it is not needed at all. I left it only for compatibility with some plugins.

 

Default boot enigma2 use DVB time is True

see m_use_dvb_time

 

https://github.com/s..._time&type=code

 

setUseDVBTime(False) need only disable iDVB time

 

https://github.com/s...BTime&type=code

[littlesat]

Then a second punt in cpp... when the current date is < year 2004 it forces transponder time here.

 

https://github.com/O...vbtime.cpp#L283

Edited by littlesat, 26 October 2023 - 17:27.

[littlesat]

That is not really not true... as when the box has an RTC it is used....

 

Default of m_use_dvb_time is indeed true,,, but set it twice on true is also not really bad.... but you can avoid initialize it of course when transponder time should be selected.

In a nutshell it is not that easy... especially when you want ntp... and the network is not 'up and running' yet.

 

Then as you can see in the post before... in cpp... when the date is unknown and you want ntp... then even there it falls back to transponder time... 

 

So still we need polling or a trigger when the network comes-up so ntp can be done to switch it to ntp as soon we have network when you select ntp.... (and then also 'auto' could be removed from the selection). And indeed 'auto' does now exactly the same as 'ntp'. Only the value 'dvb' is checked and it is checked if the network was online.

 

Yes, it’s true that it is not needed at all. I left it only for compatibility with some plugins.

 

Edited by littlesat, 26 October 2023 - 17:33.

[littlesat]

It also seems there already exist a wait for network...

 

https://github.com/O...0e9353102fd9a39

 

But it has a '15' retry timeout....

[WanWizard]

You still don't get it.

 

Your mind is so pre-occupied with something completely different that you completely miss the point here, and you go on and on about things not relevant to the problem at hand.

 

So, one last time from the top.

 

Two pieces of functionality are needed:

 

1. An option for the end-user to select which time sync mechanism is desired. This option is defined as string, with possible options:

• "dvb": only use transponder time, whether available or not
• "ntp": only use time from an external NTP source, whether available or not
• "auto", by default use ntp functionality, but if no external source is available, fall back to transponder time

2. A function / class / method that implements this functionality in RUNTIME. (note, this is important! )

 

This function needs to be called at two different moments:

1. when the user changes the preference in the configuration
2. when Enigma starts, as it is RUNTIME configuration !

In both cases, the function needs to be called with the value the user configured, i.e. the selected string value.

 

So:

 

Iissue #1: The function currently implemented doesn't reflect the defined functionality for all options (namely, "ntp" now falls back to "dvb" if no IP is present, which is both wrong and not relevant )

 

Issue #2: The function is called at startup not with the configured value, but with a not supported argument, a boolean. The fact that this somewhat and sometime works is due to the way the function is coded, and therefore pure luck.

 

Issue #3: The availability of an IP address, or lack thereof, doesn't say anything about whether or not NTP will provide the correct time, so that check is wrong.

 

These need to be addressed.

 

All the rest is fuzz and disturbance in this discussion, as it is not even defined as "nice to have".

 

So focus on addressing the problem, instead of shooting off in all sort of directions, and add more code that doesn't do what it should do.

 

edit:

 

To elabortate: the fact that an IP address is or isn't issued doesn't say anything about NTP working or not. In my case, I block all outgoing traffic on my firewall, so pool.ntp.org (which is the default) will never work for me, whether or not the box has an IP.

 

If you want to make sure NTP works, the only way to check that is manually calling ntpdate-sync and check its return values. Alternatively, you could check if the current date is 1970, if so, there was no valid date set before Enigma started.

 

But even then, if I configure "ntp", I do that because I DONT want Enigma to fall back to transponder time. If I wanted that, I would have configured "auto". So don't even think about adding code to break that functionality too !

Edited by WanWizard, 26 October 2023 - 17:52.

[Dimitrij]

Maybe simlpe

 

[StartEnigma] remove initial_call=False

Edited by Dimitrij, 26 October 2023 - 18:16.

[WanWizard]

That should indeed be removed, but doesn't fix issue #2, that still exists.

 

That issue says that timesyncChanged() is called in startEnigma with a configElement.value of False. Which due to the way timesyncChanged() is coded, causes Enigma to always use the "ntp" setting, no matter what the user actually configured.

 

Since it isn't called anywhere else, this feature is currently severly broken.

[Dimitrij]

?

def useTransponderTimeChanged(configElement):
	enigma.eDVBLocalTimeHandler.getInstance().setUseDVBTime(os.path.islink('/etc/network/if-up.d/ntpdate-sync') and False or True)


config.misc.useTransponderTime.addNotifier(useTransponderTimeChanged)

 

[Taapat]

From what I've read, I understand that Dimitrij's suggestion to add initial_call=False here is correct: https://github.com/O...rtEnigma.py#L95

 

config.misc.useTransponderTime is not used in enigma, but it cannot be removed because Dimitrij thinks that it is used somewhere else.

 

In enigma startup the correct time is set on InitUsageConfig here: https://github.com/O...tEnigma.py#L572

And for this is used the config.ntp.timesync value: https://github.com/O...ig.py#L772-L791

 

When useTransponderTimeChanged is called on startup, the time zone is not even set yet, so the time cannot be correct.

After this on eigma startup the time is changed again in initUsageConfig and the correct time is set.

 

Therefore, it is not necessary to call useTransponderTimeChanged on startup, because it does not set the correct time at that moment anyway.

It should be left for compatibility with initial_call=False so that it is not used unnecessarily in startup as Dimitrij suggests.

Edited by Taapat, 26 October 2023 - 18:40.

[littlesat]

It is eg used for the rtc…. Needs more investigation.
But there is also something that waits for network is coming up with 15 attempts…. Not 15 seconds as far I can see.
Here a lot of ‘saur kraut’ code is included.
I merged the latest merge request…. But then in ntp mode without network the stuff is running in 1970’and then the c++’stuff still uses transponder time so even this does not work at all.

Edited by littlesat, 26 October 2023 - 18:50.

[WanWizard]

 

config.misc.useTransponderTime is not used in enigma, but it cannot be removed because Dimitrij thinks that it is used somewhere else.

 

useTransponderTime is a legacy element, made obsolete by Littlesat during his NTP change, which uses a new config element.

 

Its value will always be "True", it is never set to something else, and its value is not present in enigma/settings.

 

Subsequently, due to the addNotifier() in startEnigma, timesyncChanged() is called with that "dummy" value, making it a hardcoded response, irrespectively on how you have configured the time source preferfence.

 

If you add initial_cal=False, the addNotifier() will no longer call timesyncChanged(), so nothing is set at all when Enigma starts.

 

Now, if as you say it is also called in InitUsageConfig, then all this legacy code can indeed be removed, I didn't check the code that deep. Note that it is used later on in an if in the wakeup code, so that needs updating too.

 

I don't agree with leaving legacy code in "for compatibility reasons" if you can not quantify those reasons. It is no longer used, the config key no longer exists, you can not configure it.

 

This very lengthly discussion is exactly a prime example of the confusion you can get if you leave old no longer used legacy code in place, it becomes an enormous time waster.

 

So,

 

bottom line is that merging Dimitri's PR now addresses the initial issue, a new PR is needed to remove the legacy code from startEnigma, and a new PR is needed to remove the useless IP check in timesyncChanged() and replace it with a proper "does NTP work" check.

[Taapat]

waitForNetwork is called after time is set: https://github.com/O...tEnigma.py#L585

Maybe we could check, maybe it can be called before?

[WanWizard]

It is eg used for the rtc…. Needs more investigation.
But there is also something that waits for network is coming up with 15 attempts…. Not 15 seconds as far I can see.
Here a lot of ‘saur kraut’ code is included.
I merged the latest merge request…. But then in ntp mode without network the stuff is running in 1970’and then the c++’stuff still uses transponder time so even this does not work at all.

 

STOP with staring at network code. IT IS NOT RELEVANT AND WRONG.

[WanWizard]

waitForNetwork is called after time is set: https://github.com/O...tEnigma.py#L585

Maybe we could check, maybe it can be called before?

 

This code is there to prevent a restore after flash going wrong due to a slow network init.

 

Which suggests it is in the wrong place (I initially added that in the wizard where it prompts for a restore), as for normal startups, having a network or not is not relevant.

[Taapat]

This code is there to prevent a restore after flash going wrong due to a slow network init.
 
Which suggests it is in the wrong place (I initially added that in the wizard where it prompts for a restore), as for normal startups, having a network or not is not relevant.

If we want to synchronize the time from the network, then it is relevant  .

[WanWizard]

It is not relevant in the sense that having an IP address, or even a working internet connection (i.e. you can ping or http connect), doesn't say anything about the ability to get the correct time using ntp.

 

Also, if I configure my time source as NTP instead of AUTO, I accept the fact there is no fallback to transponder time, whether or not NTP has provided a valid time or not. So there is having an IP not relevant either.

 

So, logically. the code should be

if configured "dvb" then
   enable transponder time
   disable ntp
else
   enable ntp
   if configured "auto" and ntp-didn't-work then
       enable transponder time
   else
      disable transponder time

and "ntp-didn't-work" can only be determined by actually executing ntpdate-sync and validating its response.

[Taapat]

useTransponderTime is a legacy element, made obsolete by Littlesat during his NTP change, which uses a new config element.
 
Its value will always be "True", it is never set to something else, and its value is not present in enigma/settings.

A quick search shows that config.misc.useTransponderTime is used in plugin systemtime: https://github.com/s...rTime&type=code
Of course, this plugin can be changed, but it is possible that there is another 3rd-party plugin that uses it.

[WanWizard]

The plugin systemtime has been retired in OpenPLi 8. it is also legacy, the whole point of integrating NTP in Enigma was so that the plugin was no longer needed.

 

Old Enigma only had the binary option to use transpondertime or not, so the code in startEnigma was all that was needed. Whether or not a plugin might use it is irrelevant: it not longer works, even worse, it might interfere with the current timesync setting.

 

The check in wakeup is useless too, as the value of the config element is hardcoded, and there isn't anything that can change it.

 

And to show IP connectivity isn't relevant:

root@dual:~# ping pool.ntp.org
PING pool.ntp.org (162.159.200.123): 56 data bytes
64 bytes from 162.159.200.123: seq=0 ttl=55 time=17.051 ms
^C

root@dual:~# /usr/sbin/ntpdate -v pool.ntp.org
26 Oct 19:24:52 ntpdate[11086]: ntpdate 4.2.8p15@1.3728-o Tue Apr 18 17:56:55 UTC 2023 (2)
26 Oct 19:25:01 ntpdate[11086]: no server suitable for synchronization found

while no issue with other network connectivity.