esitarski / CrossMgr

Cyclo Cross Management Application
MIT License
40 stars 20 forks source link

CrossMgrVideo trigger list sort order #112

Closed kimble4 closed 1 year ago

kimble4 commented 1 year ago

In CrossMgrVideo (current version), the triggers are listed in order, and when refreshed after a capture, the last (bottom) row is selected so that image is displayed.

Logically therefore, the list should be in ascending order by timestamp, so the bottom row contains the most recent capture. And indeed some - but not all - of the time that is the case.

However this is what I'm currently seeing on our Windows 11 timing laptop. The most recent capture is at the top of the list, and you get the first image highlighted every time it's refreshed:

screenshot

This was sorting in ascending order until about an hour ago, and now it's stuck in descending order. (I can't see any sort of UI to change the sort order.) Whatever previous versions we were running during the 2022 race season sorted in descending order too, so I assumed that the change to ascending was a recent improvement.

Meanwhile, on my Linux desktop (where I have a dev environment that might help debug this), they're sorting in ascending order, and I can't seem to reproduce this behaviour.

Looking at the code I can't see an obvious reason why this should happen...

(For anyone curious the image is of a reflective target at the end of a dark corridor that I'm bouncing an IR beam off to trigger the auto-capture. Usefully the camera can see enough IR to help line things up.)

kimble4 commented 1 year ago

Deleting the SQLite database and the CrossMgrVideo.cfg (it took me quite a while to work out that this is stored in the Registry under Windows) have no effect.

kimble4 commented 1 year ago

I think I've worked it out: It's a problem with the ListCtrl (I've confirmed that the triggers are correctly retrieved from the database and iterated over in ascending order, as you'd expect).

Looks like repeatedly inserting at the same (high) index gives different results across platforms for some reason.

113 fixes this for me.

kimble4 commented 1 year ago

No, that didn't work. It's in reverse order again today! Curiouser and curiouser...

kimble4 commented 1 year ago

ETA: Have confirmed using Append() instead of InsertItem() gives the same reverse-ordering under Windows.

Now trying with a explicit call to SortItems() (having provided a comparison function) as part of refreshTriggers(). This seems to have the desired effect (and by altering the comparison can be made to sort in either direction, which confirms it's working), but I'm not sure if it's the correct approach. Are trigger ids always in chronological order?

kimble4 commented 1 year ago

I think I've found an unrelated trigger list bug (this one is platform independent):

New captures are added to the trigger list as expected, previous triggers are unaffected.

New captures are added to the trigger list as expected, but all previous triggers are now duplicated.

'Refresh' clears the duplicates, until another capture is performed.

The list now shows a single copy of triggers for that historical day along with those from today, including the one just made. If there are intermediate days, they are not included.

It looks like it's adding all previous triggers from today where it shouldn't be, but only after a date has been selected? I've yet to work out why...

kimble4 commented 1 year ago

Okay, I think I've found the culprit:

Line 1248:

        if replace:
            self.tsMax = None

Means that it immediately forgets what the last trigger time is when a replace-refresh is performed, as when selecting a date (or clicking 'refresh', which I've confirmed also causes duplicate items to appear). As tsMax is unset, it then fetches everything after today's midnight when adding the trigger it just captured.

Commenting out that line seems to fix the problem. Not sure if that will affect anything else...

esitarski commented 1 year ago

I found this https://github.com/wxWidgets/wxWidgets/issues/14551, which states that if the LC_SORT_ASCENDING flag is used without a sort function, unexpected behavior can happen on Windows. The fix was to remove the LC_SORT_ASCENDING flag in the ListCtrl. Unfortunately, the LC_SORT_ASCENDING flag is used without a sort function in many ListCtrl examples, which makes this bug happen when you copy-and-paste code. Now fixed in the development release. Also explains why they other ListCtrls in CrossMgr work properly (they didn't use LC_SORT_ASCENDING).

I will look at the self.tsMax situation.

On Tue, Apr 4, 2023 at 3:53 PM Kim Wall @.***> wrote:

Okay, I think I've found the culprit:

Line 1248:

  if replace:
      self.tsMax = None

Means that it immediately forgets what the last trigger time is when a replace-refresh is performed, as when selecting a date (or clicking 'refresh', which I've confirmed also causes duplicate items to appear). As tsMax is unset, it then fetches everything after today's midnight when adding the trigger it just captured.

Commenting out that line seems to fix the problem. Not sure if that will affect anything else...

— Reply to this email directly, view it on GitHub https://github.com/esitarski/CrossMgr/issues/112#issuecomment-1496513465, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABGXKJWUITLTMJJOIJBKLTW7R34ZANCNFSM6AAAAAAWP6J6GI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

--

Edward Sitarski

kimble4 commented 1 year ago

Yes! That appears to have fixed it for me. And I've rebooted Windows a couple of times, mucked about with the date selector, made some edits and so on and it's still in ascending order.

Well done for finding that. All I could come up with were endless (mostly poor) tutorials showing how to use the sort mixin or similar. And yes, lots of examples with the LC_SORT_ASCENDING flag set.

esitarski commented 1 year ago

Fix in latest release.