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

Support for recorded and paused time - Modifications after 2nd code review #227

Merged
merged 10 commits into from
Sep 15, 2020

Conversation

FJBDev
Copy link
Collaborator

@FJBDev FJBDev commented Sep 14, 2020

No description provided.

@wolfgang-ch wolfgang-ch merged commit 9ea13dc into mytourbook:FJBDev Sep 15, 2020
@wolfgang-ch
Copy link
Collaborator

I like the new time icons :-)

mt-time-icons

I've not yet merged into main, first I'll do more tests

@wolfgang-ch
Copy link
Collaborator

I've found/fixed an SQL exception 2281360
and changed a mnemonic c204991

These modifications are in your FJBDev branch

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 15, 2020

I like the new time icons :-)

mt-time-icons

I've not yet merged into main, first I'll do more tests

Thanks, I like them too!
But i am still not sure that I can use them as they don't work in Linux 😧

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 15, 2020

I've found/fixed an SQL exception 2281360
and changed a mnemonic c204991

These modifications are in your FJBDev branch

Thank you, i just looked at them, that looks good to me.

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 15, 2020

@wolfgang-ch That's what happens in Linux

image

@wolfgang-ch
Copy link
Collaborator

Ubuntu is OK

mt-icons-ubuntu

@wolfgang-ch
Copy link
Collaborator

New time icon sugesstions:

  • Replaced elapsed icon with recorded icon
  • New recorded icon
  • Some suggestions for the moving icon. When searched for a new moving icon, I found this document https://www.unicode.org/L2/L2018/18090-tachograph-symbols.pdf, it is interresting to see for their process how to find a new icon, I googled for "unicode tacho"

mt-time-symbols-ubuntu-2

mt-time-symbols

Which is your preferred moving icon?

@wolfgang-ch
Copy link
Collaborator

Any unicode character could be used

mt-moving-icons-II

mt-moving-icons-II-win10

Selecting a character is easy with LibreOffice

mt-icons

@wolfgang-ch
Copy link
Collaborator

I've a lot of tours with overlapped paused times at the beginning of the tour, zooming in do not help to read the displayed values

mt-paused-time-in-tour-chart

The tour segmenter shows stacked labels which prevents overlapping, this is done in net.tourbook.ui.tourChart.ValueOverlapChecker but the whole is a relative complex code :-)

mt-tour-segmenter-with-stacked-labels

@wolfgang-ch
Copy link
Collaborator

Some tours have negative paused time

mt-negative-paused-time

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

Ubuntu is OK

mt-icons-ubuntu

Maybe it depends the desktop used. Are you using Gnome ? I am using Cinnamon

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

In GNOME,
In GNOME 3, the default font is Cantarell, Source : https://developer.gnome.org/hig/stable/typography.html.en

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

And in my Linux Debian 10.5 Cinnamon, this is what I have as a default font :
image

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

New time icon sugesstions:

* Replaced elapsed icon with recorded icon

* New recorded icon

* Some suggestions for the moving icon. When searched for a new moving icon, I found this document https://www.unicode.org/L2/L2018/18090-tachograph-symbols.pdf, it is interresting to see for their process how to find a new icon, I googled for "unicode tacho"

mt-time-symbols-ubuntu-2

mt-time-symbols

Which is your preferred moving icon?

My preferred moving icon is the one in the red square:

image

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

Any unicode character could be used

Not if it doesn't work on all the Linux 😧

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

I've a lot of tours with overlapped paused times at the beginning of the tour, zooming in do not help to read the displayed >values
The tour segmenter shows stacked labels which prevents overlapping, this is done in >net.tourbook.ui.tourChart.ValueOverlapChecker but the whole is a relative complex code :-)

I have seen this issue as well. I will look into this complex code net.tourbook.ui.tourChart.ValueOverlapChecker

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

Some tours have negative paused time

Are those tours old tours reimported ? are they new tours ? Which format are they ? FIT ? TCX ?
I have never seen this issue, this is not normal.

it's probably because the elapsed time is 1 second less than the recorded time:
image

My guess is that it's from a FIT file but the elapsed value should NOT be less than the recorded value.
I will fix that in my code but you will need to verify because I dont have such FIT files.

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 16, 2020

See my fix attempt here.
Does it fix your paused time of -1 issue ?

@wolfgang-ch
Copy link
Collaborator

And in my Linux Debian 10.5 Cinnamon, this is what I have as a default font :

I'm using Ubuntu 20.04 with it's default setup

ubuntu

@wolfgang-ch
Copy link
Collaborator

My preferred moving icon is the one in the red square:

image

Fixed

@wolfgang-ch
Copy link
Collaborator

Any unicode character could be used

Not if it doesn't work on all the Linux 😧

I would understand that 3-Byte Unicode characters, e.g. all of these moving time icons

mt-moving-icons-II

are not supported but you have choosen only 2-Byte Unicode characters

@wolfgang-ch
Copy link
Collaborator

Some tours have negative paused time

Are those tours old tours reimported ? are they new tours ? Which format are they ? FIT ? TCX ?
I have never seen this issue, this is not normal.

it's probably because the elapsed time is 1 second less than the recorded time:
image

My guess is that it's from a FIT file but the elapsed value should NOT be less than the recorded value.
I will fix that in my code but you will need to verify because I dont have such FIT files.

This issue occurred with FIT files, imported and reimported

@wolfgang-ch
Copy link
Collaborator

Statistic paused time value is larger than elapsed time value

mt-paused-time-statistic

@wolfgang-ch
Copy link
Collaborator

Missing mnemonics

mt-stat-options

@wolfgang-ch
Copy link
Collaborator

I've several tours with wrong paused time, these are mainly tours which I recorded when driving by car but I also found 1 normal bicycle tour with wrong paused time value. These tours are all recorded with an Edge 1030 -> FIT format.

mt-stat-values

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 17, 2020

Some tours have negative paused time

Are those tours old tours reimported ? are they new tours ? Which format are they ? FIT ? TCX ?
I have never seen this issue, this is not normal.
it's probably because the elapsed time is 1 second less than the recorded time:
image
My guess is that it's from a FIT file but the elapsed value should NOT be less than the recorded value.
I will fix that in my code but you will need to verify because I dont have such FIT files.

This issue occurred with FIT files, imported and reimported

Did you try with my new code available in my latest PR?

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 17, 2020

I've several tours with wrong paused time, these are mainly tours which I recorded when driving by car but I also found 1 normal bicycle tour with wrong paused time value. These tours are all recorded with an Edge 1030 -> FIT format.

mt-stat-values

I can't reproduce the issue so I'm not sure I can troubleshoot....unless you can share one or more FIT files ?

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 17, 2020

And in my Linux Debian 10.5 Cinnamon, this is what I have as a default font :

I'm using Ubuntu 20.04 with it's default setup

Ok, so it shows that you are using GNOME 3 on your Ubuntu, so maybe it's because GNOME and Cinnamon use a different font ?

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 17, 2020

Missing mnemonics

mt-stat-options

Fixed

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 17, 2020

Any unicode character could be used

Not if it doesn't work on all the Linux 😧

I would understand that 3-Byte Unicode characters, e.g. all of these moving time icons

mt-moving-icons-II

are not supported but you have choosen only 2-Byte Unicode characters

Maybe it's the default font used by GNOME and Cinnamon ?
I will continue to investigate.

@wolfgang-ch
Copy link
Collaborator

This PR is now merged into main, it was a long reviewing time.

Some more issues which I found:

  • Unit column should show only units

mt-column-units

  • One missing mnemonic

mt-mnemonic

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 18, 2020

Some more issues which I found:

* Unit column should show only units

This is what is in the official 20.8 so I thought it was normal to have the unit the same as the column header:
image

I can fix

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 18, 2020

* One missing mnemonic

mt-mnemonic

Fixed

@wolfgang-ch
Copy link
Collaborator

wolfgang-ch commented Sep 18, 2020

This is what is in the official 20.8 so I thought it was normal to have the unit the same as the column header:
image

I can fix

It looked a bit strange when I first saw it and why I created this "issue". So we can keep it except "Recorded time" which shows a text

@FJBDev
Copy link
Collaborator Author

FJBDev commented Sep 18, 2020

It looked a bit strange when I first saw it and why I created this "issue". So we can keep it except "Recorded time" which shows a text

Whatever you prefer but actually the menu is very confusing to me because I don't understand the difference between the "Tour Format" column and the "Unit" column. It seems they are the same to me ?

image

@wolfgang-ch
Copy link
Collaborator

  • Tour Format: how values are formatted

  • Unit: when searching references then you can see where units are used

mt-columns

@wolfgang-ch
Copy link
Collaborator

See my fix attempt here.
Does it fix your paused time of -1 issue ?

YES !

Old
image

New

mt-negative-pause

@FJBDev
Copy link
Collaborator Author

FJBDev commented Oct 7, 2020

I've a lot of tours with overlapped paused times at the beginning of the tour, zooming in do not help to read the displayed values

mt-paused-time-in-tour-chart

The tour segmenter shows stacked labels which prevents overlapping, this is done in net.tourbook.ui.tourChart.ValueOverlapChecker but the whole is a relative complex code :-)

mt-tour-segmenter-with-stacked-labels

Implemented the overlap for TourChart pauses in my recent PR

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

Successfully merging this pull request may close these issues.

None yet

2 participants