-
Notifications
You must be signed in to change notification settings - Fork 231
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
new Poller #1006
Comments
I think the hang when flashing or rebooting could also be due to the amount of logging messages. |
Hey @Dimitrie78 - could you please write a clear statement of the problem. What is happening exactly!? The old poller should also have sent the query after 3600 seconds afaict. I certainly did not see the mechanism for sending earlier, so that surprises me that it did that. Too many logs could certainly be a problem. I might have to split it up into a couple of areas of logs to facilitate debugging. |
the old Poller send first all polls. |
AH. Found the issue with the poller starting at 1. Makes sense now. Fix
coming.
//.ichael
…On Sun, 28 Apr 2024 at 21:35, Dimitrie78 ***@***.***> wrote:
the old Poller send first all polls.
{ 0x7E5, 0x7ED, VEHICLE_POLL_TYPE_OBDIIEXTENDED, 0x1001, {
0,3600,3600,3600 }, 0, ISOTP_STD }, // Variant coding
when i set PollSetState(0) it never run. on PollSetState(1) it poll first
an then after 3600 sec. But now it poll after 3600 sec the first time.
—
Reply to this email directly, view it on GitHub
<#1006 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANWLSZPGNPYNUIUN6FLY7T3KJAVCNFSM6AAAAABG47N7YKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRGQ4DOMRZGM>
.
You are receiving this because you were mentioned.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/issues/1006/2081487293@
github.com>
|
after a short test it sems to work now |
I was probably happy too soon. it doesn't work properly yet. it still doesn't start like before Old Poller------------------ New Poller----------------- |
Yeah that was part of the problem.
Looking at another solution but possibly also at reverting the patch .
Michael
…On Tue, 30 Apr 2024, 03:44 Dimitrie78, ***@***.***> wrote:
I was probably happy too soon. it doesn't work properly yet. it still
doesn't start like before
Old Poller------------------
OVMS# wakeup
Vehicle has been woken
I (111097) v-smarted: Car has woken (CAN bus activity)
D (111707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=F111],
ticker=0, wait=0, cnt=0/5
D (111747) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=226],
ticker=0, wait=0, cnt=1/5
D (111837) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=225],
ticker=0, wait=0, cnt=2/5
D (111957) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=22A],
ticker=0, wait=0, cnt=3/5
D (112017) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=223],
ticker=0, wait=0, cnt=4/5
D (112707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=F190],
ticker=0, wait=0, cnt=0/5
D (112757) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=310],
ticker=0, wait=0, cnt=1/5
D (113497) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=203],
ticker=0, wait=0, cnt=2/5
D (113497) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=304],
ticker=0, wait=0, cnt=3/5
D (113527) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=F18C],
ticker=0, wait=0, cnt=4/5
D (113707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=F150],
ticker=0, wait=0, cnt=0/5
D (113727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=F151],
ticker=0, wait=0, cnt=1/5
D (113807) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=201],
ticker=0, wait=0, cnt=2/5
D (113837) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=202],
ticker=0, wait=0, cnt=3/5
D (113957) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=30B],
ticker=0, wait=0, cnt=4/5
D (114707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=30C],
ticker=0, wait=0, cnt=0/5
D (114717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=D000],
ticker=0, wait=0, cnt=1/5
D (114727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=204],
ticker=0, wait=0, cnt=2/5
D (114747) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=209],
ticker=0, wait=0, cnt=3/5
D (114767) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=1001],
ticker=0, wait=0, cnt=4/5
D (115707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=2047],
ticker=0, wait=0, cnt=0/5
D (115717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=230A],
ticker=0, wait=0, cnt=1/5
D (115717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=2308],
ticker=0, wait=0, cnt=2/5
D (115727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=2309],
ticker=0, wait=0, cnt=3/5
D (115727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=D032],
ticker=0, wait=0, cnt=4/5
D (116707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=6309],
ticker=0, wait=0, cnt=0/5
D (116717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=D041],
ticker=0, wait=0, cnt=1/5
D (116717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=630A],
ticker=0, wait=0, cnt=2/5
D (116727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=6321],
ticker=0, wait=0, cnt=3/5
D (116727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=D302],
ticker=0, wait=0, cnt=4/5
D (117707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=6303],
ticker=0, wait=0, cnt=0/5
D (117717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=2041],
ticker=0, wait=0, cnt=1/5
D (117717) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=2043],
ticker=0, wait=0, cnt=2/5
D (117727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=6308],
ticker=0, wait=0, cnt=3/5
D (117727) vehicle-poll: PollerSend(SRX)[1]: entry at[type=22, pid=207],
ticker=0, wait=0, cnt=4/5
D (118707) vehicle-poll: PollerSend(PRI)[1]: entry at[type=22, pid=208],
ticker=0, wait=0, cnt=0/5
D (119407) vehicle-poll: PollerSend(SRX): cycle complete for ticker=0
D (119707) vehicle-poll: PollerSend(PRI): cycle complete for ticker=1
D (120707) vehicle-poll: PollerSend(PRI): cycle complete for ticker=2
D (121707) vehicle-poll: PollerSend(PRI): cycle complete for ticker=3
New Poller-----------------
OVMS# wakeup
Vehicle has been woken
I (40978) v-smarted: Car has woken (CAN bus activity)
D (40978) vehicle-poll: Pollers: Queue SetState()
D (40978) vehicle-poll: Pollers: PollState(1)
D (41818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for
ticker=3601
D (41818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (42818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=2
D (42818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (43818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=3
D (43818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (44818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=4
D (44818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (45818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=5
D (45818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (46818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=6
D (46818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (47818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=7
D (47818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (48818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=8
D (48818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (49818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for ticker=9
D (49818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
D (50818) vehicle-poll: [1]PollerNextTick(PRI): cycle complete for
ticker=10
D (50818) vehicle-poll: PollSeriesList::NextPollEntry[!v.standard]:
ReachedEnd
—
Reply to this email directly, view it on GitHub
<#1006 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANWMK5CYSLTSY5JVATDY72PLBAVCNFSM6AAAAABG47N7YKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBTGUZDSNZWGM>
.
You are receiving this because you were mentioned.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/issues/1006/2083529763@
github.com>
|
@Dimitrie78 That should possibly work now with some changes to logging from @dexterbg and fixes to which task runs the virtual RX methods. |
Just had another small P/R accepted which should help keep the poller from interfering in messages on the bus. I have a couple more ideas to help diagnose further issues. |
I tested it on a test module. But I still don't have the old order to test it in my car. |
I did not understand all of that. I will use clear sentences that are
easier to translate.
There is one more 'pull request' that might help if further diagnosis is
needed.
I am determined to make this work. I believe we are close to doing so.
Michael
…On Sun, 12 May 2024, 16:59 Dimitrie78, ***@***.***> wrote:
I tested it on a test module.
It seems more stable and no longer hangs when restarting and flashing.
But I still don't have the old order to test it in my car.
First work through the list and then according to time.
—
Reply to this email directly, view it on GitHub
<#1006 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANSUFDEI32FJQW57T6TZB4VNNAVCNFSM6AAAAABG47N7YKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBWGE3TEOJXGA>
.
You are receiving this because you were mentioned.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/issues/1006/2106172970@
github.com>
|
Michael, the remaining issue here is this: the poller is supposed to do an initial run for all defined polls, whenever a new poll list is installed or the state is changed. The old poller achieved that by starting the ticker at 0, which results in all modulo tests becoming true. To only do this initial full poll run once, the ticker cycles over from 3601 to 1. The current code starts the ticker at 3601, which then cycles over to 1, so the poller never does the initial full run. Without the initial run, a request defined to run e.g. once every 120 seconds, will first be sent after 120 seconds. That is wrong and leads to strange delays for some metric state/change detections. I'm seeing issues from this on the UpMiiGo as well. Dimitrie told me he's already tried setting Regards, |
Oh. Damn. that was meant to roll over from 3601 to 0.
I'll make sure that works, sorry I hadn't received/noticed the feedback
that it didn't work.
//.ichael
…On Mon, 20 May 2024 at 00:26, Michael Balzer ***@***.***> wrote:
Michael, the remaining issue here is this: the poller is supposed to do an
initial run for all defined polls, whenever a new poll list is installed or
the state is changed. The old poller achieved that by starting the ticker
at 0, which results in all modulo tests becoming true. To only do this
initial full poll run once, the ticker cycles over from 3601 to 1.
The current code starts the ticker at 3601, which then cycles over to 1,
so the poller never does the initial full run.
Without the initial run, a request defined to run e.g. once every 120
seconds, will first be sent after 120 seconds. That is wrong and leads to
strange delays for some metric state/change detections. I'm seeing issues
from this on the UpMiiGo as well.
Dimitrie told me he's already tried setting const init_ticker = 0, but
that didn't work as expected. Probably some of the sub second ticker stuff
gets in the way there.
Regards,
Michael
—
Reply to this email directly, view it on GitHub
<#1006 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANRVZ6QZBMDPOJBHLELZDDHCTAVCNFSM6AAAAABG47N7YKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZGI4TEMZXG4>
.
You are receiving this because you were mentioned.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/issues/1006/2119292377@
github.com>
|
Oooh. Missed this subtlety .. I have a fix. This is the original:
m_poll.ticker++;
if (m_poll.ticker > 3600) m_poll.ticker -= 3600;
Which means.. it starts at 0 ... then goes up to 3600 and then goes back
down to 1 and then up to 3600.
So that 0 state is only right at the beginning.
I could make it like this:
if (m_poll.ticker < max_ticker)
m_poll.ticker++;
else
m_poll.ticker = (m_poll.ticker == init_ticker) ? 0 : 1;
There's no need for -=. This is always called in the same task now.
//
On Mon, 20 May 2024 at 08:47, Michael Geddes ***@***.***>
wrote:
… Oh. Damn. that was meant to roll over from 3601 to 0.
I'll make sure that works, sorry I hadn't received/noticed the feedback
that it didn't work.
//.ichael
On Mon, 20 May 2024 at 00:26, Michael Balzer ***@***.***>
wrote:
> Michael, the remaining issue here is this: the poller is supposed to do
> an initial run for all defined polls, whenever a new poll list is installed
> or the state is changed. The old poller achieved that by starting the
> ticker at 0, which results in all modulo tests becoming true. To only do
> this initial full poll run once, the ticker cycles over from 3601 to 1.
>
> The current code starts the ticker at 3601, which then cycles over to 1,
> so the poller never does the initial full run.
>
> Without the initial run, a request defined to run e.g. once every 120
> seconds, will first be sent after 120 seconds. That is wrong and leads to
> strange delays for some metric state/change detections. I'm seeing issues
> from this on the UpMiiGo as well.
>
> Dimitrie told me he's already tried setting const init_ticker = 0, but
> that didn't work as expected. Probably some of the sub second ticker stuff
> gets in the way there.
>
> Regards,
> Michael
>
> —
> Reply to this email directly, view it on GitHub
> <#1006 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AABNANRVZ6QZBMDPOJBHLELZDDHCTAVCNFSM6AAAAABG47N7YKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZGI4TEMZXG4>
> .
> You are receiving this because you were mentioned.Message ID:
> <openvehicles/Open-Vehicle-Monitoring-System-3/issues/1006/2119292377@
> github.com>
>
|
If you need to avoid starting the ticker at 0 (?), that might do as an alternative. But with this logic in |
Yeah this should work.
Part of the issue was moving the poll tick from after the poll to before
the poll. This was part of a larger choice.
So anyway, the first Primary Poll after a reset will fall through with
ticker = 0.
Michael
…On Mon, 20 May 2024, 14:42 Michael Balzer, ***@***.***> wrote:
If you need to avoid starting the ticker at 0 (?), that might do as an
alternative. But with this logic in PollerNextTick(), take care to verify
the initial poll run is done without an additional 1 second delay, i.e. on
the following ticker event after setting the list or state.
—
Reply to this email directly, view it on GitHub
<#1006 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNANUO5PIDZGHZ5YAZBMLZDGLMTAVCNFSM6AAAAABG47N7YKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZG43TIOBZGU>
.
You are receiving this because you were mentioned.Message ID:
<openvehicles/Open-Vehicle-Monitoring-System-3/issues/1006/2119774895@
github.com>
|
@frogonwheels
The main problem with the new poller is that it only sends the query after the time has expired.
Not like before, first send then wait for time.
This means that a query that is set to 3600 seconds will only be queried for the first time after 60 minutes. and not immediately.
It also happens that the module is no longer accessible after flashing and you have to pull the plug first. But that's random
The text was updated successfully, but these errors were encountered: