Simplified Equipment feature (for feedback) #4466

paulj49457 · 2024-04-05T11:51:46Z

This PR adds an equipment maintenance feature to GC, any feedback or guidance on future development would be useful.

Design Aims:

• To be as generic as possible, support multiple equipment & components
• To minimise the number of Metadata items added to GC activities
• Minimise impact to existing GC code.

Description
This PR will create a new unpopulated “EquipmentLink” metadata item in all activities to provide a mechanism to link an activity to an equipment link item defined in the new page. The diagram below tries to illustrate the relationships between the equipment items

Within the equipment pages, various user defined components can be attached to the equipment link with time-spans of use, allowing the calculation of distances covered by individual components. The equipment page contains an equipment List:

Popup Menus

The Equipment List menu allows:
• Distance Recalculation
• Add Equipment Link
• Load/Save Equipment
• Expand/Collapse All

The Equipment List context menu (right click on an equipment item) allows:
• Add Shared
• Add Distance
• Add Time
• Add Banner
• Delete*
• Top**
• Shift Up/Down**
• Bottom**
• Shift Left**

Note:
*: Items with children cannot be deleted, move their children (drag & drop them), or delete their children.
**: These options are available when relevant.

Equipment List –> Shared Equipment

Items which are moved or shared across equipment (wheels, etc) should be created in this section, and then the items dragged to the relevant Equipment link headers. This will cause a reference to be created that will accumulate any distance and added to the shared equipment item.
{} – indicates the number of references to this object
[] – indicates the total mileage of the equipment (from its references)

Shared Distance Equipment Page:
• Description: Equipment description
• nonGCdistance: allows an initial value to be set for activities not recorded within GC
• gcDistance: the distance contribution of all the relevant GC activities
• Total distance: the total of nonGCdistance + gcDistance
• Replacement Distance: Sets the replacement distance warning, set a value of zero to disable warnings.

• Note: Save must be pressed to store any text/date changes

Equipment List –> Equipment Links

Equipment links “connect” to the activities.
All equipment items within an equipment hierarchy will accumulate the equipment links distance, allowing for any time restrictions.
() – indicates the number of “connected” activities
[] – indicates the total mileage of the “connected” activities.

Equipment Link Page:
• Equipment Link: text that matches the Activities EquipmentLink Metadata item.
• Activities: the number of “connected” activities.
• gcDistance: the distance contribution of all the “connected” activities.

Note: Save must be pressed to store any text/date changes

Equipment List - Distance Equipment:

Distance equipment supports the management of items which wear out, chains, cassettes, tyres, running shoes, etc that are not shared. This entry is drawn in red text when the replacement distance is exceeded.
[] – indicates the total mileage of the equipment (allowing for time restrictions).

Distance Equipment Page:
• Description: Equipment description
• nonGCdistance: allows an initial value to be set for activities not recorded within GC
• gcDistance: the distance contribution of all the relevant GC activities
• Total distance: the total of nonGCdistance + gcDistance
• Replacement Distance: Sets the replacement distance warning, set a value of zero to disable warnings.
• Start Date: allows a start date when the equipment was first used to be set
• End Date: allows an end date when the equipment was last used to be set

• Note: Save must be pressed to store any text/date changes

Equipment List - Time equipment:

Time equipment supports reminders for bike service intervals, hydraulic brake fluid changes, etc. This entry is drawn in red text when the replacement is overdue.

< > - indicates the number days remaining.

Time Equipment Page:
• Description: Equipment description
• Start Date: allows an initial date to be set
• Replacement Date: Sets the replacement date warning.

Note: Save must be pressed to store any text/date changes

Equipment List – Equipment References

These are created by dragging a shared distance item (from under shared Equipment banner) and dropping them on any item within an Equipment Link’s hierarchy. They accumulate any distance (allowing for time restrictions) of the equipment link and this is added to the shared distance items total.
[ ] -- indicates the total mileage of the equipment reference (allowing for time restrictions).

Equipment Reference Page
• Description: Equipment description
• Reference Distance: the distance contribution of all the relevant GC activities
• Start Date: allows a start date when the equipment was first used to be set
• End Date: allows an end date when the equipment was last used to be set

Note: Save must be pressed to store any text/date changes

Notes:
• There is no automatic recalculation of distances, this can be initiated from either the Equipment List menu, distances depend on your GC settings. With 2800 activities it takes less than a second to recalculate (using only 1/4 of the machines cores).
• Equipment settings are saved on closing Goldencheetah, they can also be save/loaded should you decide you want to undo a set of recent changes. The settings are store in the athlete’s config directory in equipments.xml, be careful if you want to directly edit this file.
• This PR only defines & calculates distances for the current athletes’ activities.
• Instead of setting your equipment link to a bicycle (or similar) you could set all activities of one type with the same equipment link name (say running), then use the time spans to allocate distance to specific running shoes.

Please experiment with it and let me know your thoughts….

thejockl · 2024-04-09T14:34:22Z

To make your code compile on Linux (gcc), I had to do 2 modifications:

Charts/EquipmentWindow.cpp:192:28: error: expected primary-expression before ‘struct’ (multiple additional lines)
- put braces around struct widgetMapType: pageWids.push_back((struct widgetMapType) { new QLabel(), new QLabel(tr("Distance Equipment")), false });
- use widgetMapType without struct: pageWids.push_back(widgetMapType { new QLabel(), new QLabel(tr("Distance Equipment")), false });
Core/EquipmentModel.cpp:412:26: error: could not convert ‘eqNodeType::EQ_LINK’ from ‘eqNodeType’ to ‘int’
- workaround: make eqNodeType a enum instead of enum class - or better change the parameters typ

Also your code uses tabs instead of 4 spaces for indentation

Now I will play with the functionality and come back later

paulj49457 · 2024-04-09T16:10:44Z

Thank you I'll look to fix that for Linux, seems to work fine on Windows 11 & Visual studio 2022 compiler. Unfortunately I've just upgraded to the latest commit, which includes the SplashScreen PR which is causing GC to crash, I'm using Qt5.15.

thejockl · 2024-04-09T17:00:52Z

I also get a crash on exit since on your branch (I still have it prior to splashscreen, commit ec59c09):

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007fddfcaa81cf in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  0x00007fddfca5a472 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007fddfca444b2 in __GI_abort () at ./stdlib/abort.c:79
#4  0x00007fddfcca0a3d in  () at /lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007fddfccb1f4a in  () at /lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007fddfcca05e9 in std::unexpected() () at /lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x00007fddfccb2cc3 in  () at /lib/x86_64-linux-gnu/libstdc++.so.6
#8  0x000055ae3a173d3c in AbstractView::saveState() (this=this@entry=0x55ae3cd5b180) at Gui/AbstractView.cpp:274
#9  0x000055ae3a17446a in AbstractView::~AbstractView() (this=0x55ae3cd5b180, __in_chrg=<optimized out>) at Gui/AbstractView.cpp:92
#10 0x000055ae3a179829 in AnalysisView::~AnalysisView() (this=0x55ae3cd5b180, __in_chrg=<optimized out>) at Gui/Views.cpp:67
#11 0x000055ae3a16d4cb in AthleteTab::~AthleteTab() (this=0x55ae3cb11560, __in_chrg=<optimized out>) at Gui/AthleteTab.cpp:115
#12 0x000055ae3a16d539 in AthleteTab::~AthleteTab() (this=0x55ae3cb11560, __in_chrg=<optimized out>) at Gui/AthleteTab.cpp:121
#13 0x000055ae3a11bc83 in MainWindow::removeAthleteTab(AthleteTab*) (this=this@entry=0x55ae3c708ad0, tab=tab@entry=0x55ae3cb11560) at Gui/MainWindow.cpp:2199
#14 0x000055ae3a11c373 in MainWindow::closeEvent(QCloseEvent*) (this=0x55ae3c708ad0, event=0x7ffd93025790) at Gui/MainWindow.cpp:1180
#15 0x00007fde063a5dc8 in QWidget::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
...

paulj49457 · 2024-04-09T18:17:02Z

ok, thanks, I'll take a look at this exit crash, probably a side effect as another view has been added.

I've identified the start-up splash screen issue I was having: #4471

paulj49457 · 2024-04-09T20:12:49Z

Joachim the exit crash appears to be caused by the following line:

#8 0x000055ae3a173d3c in AbstractView::saveState() (this=this@entry=0x55ae3cd5b180) at Gui/AbstractView.cpp:274

which has the line of code: QString view(viewName());

viewName() is a pure virtual function, which all the views must implement and return their textual name, but it would appear that this has cause problems for the compiler, as it's within the QString constructor. I'll change the code to read:

QString view = viewName();

Hopefully this will prevent the exit crash.

paulj49457 · 2024-04-09T20:59:06Z

The latest commit (7f9bf58) has the following fixed (from Joachim's comments above):

Within EquipmentWindow.cpp widgetMapType is now used without struct: pageWids.push_back(widgetMapType { new QLabel(), new QLabel(tr("Distance Equipment")), false });
Core/EquipmentModel.cpp:412:26: error: could not convert ‘eqNodeType::EQ_LINK’ from ‘eqNodeType’ to ‘int’ this has now been static-cast to the correct type.
Change to AbstractView to prevent the exit crash reported.
Corrected all the files to remove tabs and use 4 spaces for indentation.

thejockl · 2024-04-10T16:20:58Z

A little glitch made it into src.pro: Gui/PerspectiveDialog is named twice (both for .h and .cpp), leading to linker errors because of duplicate symbols.

Also I get warnings because of initialization order (probably not bad but polluting the output):

Core/EquipmentModelManager.h: In constructor ‘EquipmentParser::EquipmentParser(QVector<flatEqNode>&, QDateTime&, bool&, uint32_t&)’:
Core/EquipmentModelManager.h:65:30: warning: ‘EquipmentParser::flatEqNodes_’ will be initialized after [-Wreorder]
   65 |         QVector<flatEqNode>& flatEqNodes_;
      |                              ^~~~~~~~~~~~
Core/EquipmentModelManager.h:64:20: warning:   ‘QDateTime& EquipmentParser::lastRecalcRef_’ [-Wreorder]
   64 |         QDateTime& lastRecalcRef_;
      |                    ^~~~~~~~~~~~~~
Core/EquipmentModelManager.h:50:9: warning:   when initialized here [-Wreorder]
   50 |         EquipmentParser(QVector<flatEqNode>& flatEqNodes, QDateTime& lastRecalcRef,
      |         ^~~~~~~~~~~~~~~
Core/EquipmentModelManager.h:64:20: warning: ‘EquipmentParser::lastRecalcRef_’ will be initialized after [-Wreorder]
   64 |         QDateTime& lastRecalcRef_;
      |                    ^~~~~~~~~~~~~~
Core/EquipmentModelManager.h:63:15: warning:   ‘bool& EquipmentParser::metricUnitsRef_’ [-Wreorder]
   63 |         bool& metricUnitsRef_;
      |               ^~~~~~~~~~~~~~~
Core/EquipmentModelManager.h:50:9: warning:   when initialized here [-Wreorder]
   50 |         EquipmentParser(QVector<flatEqNode>& flatEqNodes, QDateTime& lastRecalcRef,
      |         ^~~~~~~~~~~~~~~
Core/EquipmentModelManager.h:63:15: warning: ‘EquipmentParser::metricUnitsRef_’ will be initialized after [-Wreorder]
   63 |         bool& metricUnitsRef_;
      |               ^~~~~~~~~~~~~~~
Core/EquipmentModelManager.h:62:19: warning:   ‘uint32_t& EquipmentParser::versionRef_’ [-Wreorder]
   62 |         uint32_t& versionRef_;
      |                   ^~~~~~~~~~~
Core/EquipmentModelManager.h:50:9: warning:   when initialized here [-Wreorder]
   50 |         EquipmentParser(QVector<flatEqNode>& flatEqNodes, QDateTime& lastRecalcRef,
      |         ^~~~~~~~~~~~~~

thejockl · 2024-04-10T20:03:34Z

The crash on close is still there, coming with the message

pure virtual method called
terminate called without an active exception

The crash also happens if I use a fixed string in AbstractView::saveState() for view. This points towards use of deleted objects, I will try debugging into this.

paulj49457 · 2024-04-11T06:52:12Z

Joachim, I understand the crash issue, calling virtual functions for derived classes in destructors doesn't appear to be well defined (see link), so the Visual Studio c++ compiler is ok with it, but the Linux (gcc) one isn't, so I'll re-structure that piece of code.

https://stackoverflow.com/questions/12092933/calling-virtual-function-from-destructor

paulj49457 · 2024-04-11T16:28:36Z

Joachim, would you mind testing the latest commit 56180f9 to hopefully resolve the exit crash

thejockl · 2024-04-12T08:58:31Z

Unfortunately, now I get another crash on exit (SEGV):

#0  0x000055cd48c19e1e in EquipmentModelManager::~EquipmentModelManager() (this=0x55cd4b2ba790, __in_chrg=<optimized out>) at Core/EquipmentModelManager.cpp:52
#1  0x000055cd48c19ec9 in EquipmentModelManager::~EquipmentModelManager() (this=0x55cd4b2ba790, __in_chrg=<optimized out>) at Core/EquipmentModelManager.cpp:53
#2  0x000055cd48dbc9be in MainWindow::~MainWindow() (this=0x55cd4aed7bb0, __in_chrg=<optimized out>) at Gui/MainWindow.cpp:1196
#3  0x000055cd48dbca79 in MainWindow::~MainWindow() (this=0x55cd4aed7bb0, __in_chrg=<optimized out>) at Gui/MainWindow.cpp:1197
#4  0x00007f197caf970b in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#5  0x00007f1985b62f32 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#6  0x00007f197cacc748 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#7  0x00007f197cacfe51 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#8  0x00007f197cb28697 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#9  0x00007f197a6c31f4 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007f197a6c6317 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#11 0x00007f197a6c6930 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00007f197cb27d4a in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#13 0x00007f197cacb0fb in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#14 0x00007f197cad38a4 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#15 0x000055cd487f5488 in main(int, char**) (argc=<optimized out>, argv=<optimized out>) at Core/main.cpp:778

paulj49457 · 2024-04-13T23:08:10Z

I can now replicate the crash on my Windows VS setup and the exit crash should now be fixed in the latest commit.

peret2000 · 2024-04-16T17:36:24Z

Congrats for the job and many thanks. I will use it to see how it suits my standard usage and let you know if I find ideas regarding functionality, in case they are worth and make sense.
As my first use, I have found an issue when having two athletes opened: the switch from one athete to the other does not seem to work with Equipment page. The first time you go to Equipment page it works but afterwards, any change of user or page (activities/train...) makes you cannot come back to Equipment. I guess it is some kind of pages control that lacks in this one, but I ignore that core part of the code.

paulj49457 · 2024-04-16T21:54:56Z

Hi @peret2000, the current implementation only supports a single athlete, as the equipment.xml file is saved to athletes configuration directory, and to calculate the distances from more than one athlete's activities would be more complex.

I think if you switch athletes using the tab and then load the new athletes equipment file from the drop down menu, it should work. Remember to save any updates to equipment before switching to the next athlete and then reload the new athletes equipment file.

Thank you for the feedback, and I'll have think about the multi athlete problem

…ive file

paulj49457 force-pushed the equipment_feature_simple branch from ec59c09 to 7f9bf58 Compare April 9, 2024 20:56

paulj49457 force-pushed the equipment_feature_simple branch from e7d6d89 to 6f52df9 Compare April 11, 2024 06:11

paulj49457 force-pushed the equipment_feature_simple branch from d0ecd34 to 100749a Compare May 4, 2024 14:51

paulj49457 added 6 commits May 10, 2024 08:18

Initial version of the Simplified Equipment feature

0a320e5

fix for exit crash due to virtual func call in destructor

819eb73

fixes the exit crash and removes view specific code from AbstractView

ab24f33

Correction to load the qt resources version of the equipment perspect…

f868372

…ive file

Add make & break equipment reference functionality

f09b2aa

Addtional recalc button

3a74d15

paulj49457 force-pushed the equipment_feature_simple branch from 7c0b250 to 3a74d15 Compare May 10, 2024 07:25

paulj49457 added 4 commits May 24, 2024 09:08

merged master

5499747

Merge branch 'master' into equipment_feature_simple

78937cf

Merge branch 'master' into equipment_feature_simple

c909f94

Improved navigator colors + recalc button position

de58cfe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplified Equipment feature (for feedback) #4466

Simplified Equipment feature (for feedback) #4466

paulj49457 commented Apr 5, 2024

thejockl commented Apr 9, 2024

paulj49457 commented Apr 9, 2024

thejockl commented Apr 9, 2024

paulj49457 commented Apr 9, 2024

paulj49457 commented Apr 9, 2024

paulj49457 commented Apr 9, 2024 •

edited

thejockl commented Apr 10, 2024

thejockl commented Apr 10, 2024

paulj49457 commented Apr 11, 2024

paulj49457 commented Apr 11, 2024

thejockl commented Apr 12, 2024

paulj49457 commented Apr 13, 2024

peret2000 commented Apr 16, 2024

paulj49457 commented Apr 16, 2024

Simplified Equipment feature (for feedback) #4466

Are you sure you want to change the base?

Simplified Equipment feature (for feedback) #4466

Conversation

paulj49457 commented Apr 5, 2024

thejockl commented Apr 9, 2024

paulj49457 commented Apr 9, 2024

thejockl commented Apr 9, 2024

paulj49457 commented Apr 9, 2024

paulj49457 commented Apr 9, 2024

paulj49457 commented Apr 9, 2024 • edited

thejockl commented Apr 10, 2024

thejockl commented Apr 10, 2024

paulj49457 commented Apr 11, 2024

paulj49457 commented Apr 11, 2024

thejockl commented Apr 12, 2024

paulj49457 commented Apr 13, 2024

peret2000 commented Apr 16, 2024

paulj49457 commented Apr 16, 2024

paulj49457 commented Apr 9, 2024 •

edited