Maproom / qmapshack

Consumer grade GIS software
GNU General Public License v3.0
271 stars 63 forks source link

[QMS-623] remove use of QTimer in brouter startup error detection #632

Closed ntruchsess closed 10 months ago

ntruchsess commented 10 months ago

What is the linked issue for this pull request:

QMS-#623

What you have done:

to distinguish normal shutdown from errors during startup a timer was used. Any transition to processState::NotRunning that would happen after expiry of this timer would not result in showing the error but would be ignored silently. The timer has been replaced by a boolean flag 'isBeingStopped' that is set to false on process startup and to true when klicking the stop-button.

Steps to perform a simple smoke test:

  1. make sure BRouter is installed locally.
  2. validate local BRouter starts fine by clicking the start/stop-button to the right of 'BRouter: stopped'
  3. validate 'BRouter: stopped' has changed to 'BRouter: running' and that a second button did appear to the right of the start-button.
  4. stop local BRouter by clicking the start/stop-button. Now 'BRouter: stopped' should be displayed again.
  5. while QMapShack is still running locate the brouter-jar-file and rename it or move it to a different location.
  6. now click the BRouter start/stop button again. => the label should switch to 'BRouter: starting' respectivly 'BRouter running' for a very short time and finally show 'BRouter stopped' again. Below the start/stop-button an error-message and a 'Dismiss'-button should be visible.

Does the code comply to the coding rules and naming conventions Coding Guidelines:

Is every user facing string in a tr() macro?

Did you add the ticket number and title into the changelog? Keep the numeric order in each release block.

wthaem commented 10 months ago

I merged ntruchsess/QMS-630 and ntruchsess/QMS-623 into kiozen/QMS-624 (I hope correctly). I could see some change in the behavior of the log window but also a crash with the same call stack as before.

ntruchsess commented 10 months ago

@kiozen conflict is resolved, PR is ready to merge