[AlainKnaff]

📝 Pull Request Title

Button for starting / stopping track recording on map screen

🛠️ Issue

• Closes: none

🛠️ Related issues (if applicable)

• ISSUES: none

📖 Description

This provides an addition button on the map screen, on top of center, zoom in and zoom out.

The new button show a green circle while track recording is on, and a red circle if it is stopped. If another track than the currently shown is "active", the button is not shown.

Pressing the button toggles it, i.e. if the track was previously recording it will now be stopped, and vice versa

This is convenient, as now you don't need to click all your way back to the Track Logger or even Track Manager activity to stop or start recording.

🖼️ Screenshots (if applicable)

• Include any relevant screenshots or examples of the changes made. This helps reviewers visualize the updates

✅ Pull Request Checklist

• The PR is proposed to the DEVELOP branch.
• The changes have been tested on the target Android API and minimum Android API.
• Automated tests have been added (if applicable).
• The feature is well documented.
• There is a reference to the original ISSUE and related work.

📝 Additional Notes

• Any additional context, questions, or considerations for the reviewers.
• Take a look at our Contributing Guide before submitting your PR.

[jamescr]

A quick way to start/stop recording from the map is a very useful enhancement, and it seems to align with the direction of the UX/UI audit discussed in #690. (The Figma file linked in the discussion contains the visual proposals in English).

Before we can consider this PR, a few points need to be addressed. The PR introduces GPSLoggerConnectionListener extends Activity and makes DisplayTrackMap and TrackLogger inherit from it. I'm cautious about this: TrackLogger is already planned for migration to AppCompatActivity (as a continuation of #713), and the intermediate Activity subclass would complicate that migration.

Also, GPSLogger already handles INTENT_START_TRACKING and INTENT_STOP_TRACKING in its BroadcastReceiver. Using only the existing broadcasts would be more consistent with the current pattern in GPSLogger. It would also be great if the start/stop logic could work generally, not only from the map view. An alternative worth considering is reusing the existing gpsstatus_record.xml layout (or a simplified version) in DisplayTrackMap or adding a menu item.

The PR also lacks automated tests, which are important given that it touches the lifecycle of two Activities and the start/stop flow.

To keep the discussion organized, could you open an issue describing the feature and the proposed approach first? That would give us room to agree on the UI changes and the communication mechanism with GPSLogger before we land the implementation. Thanks again for the contribution.

[AlainKnaff]

A quick way to start/stop recording from the map is a very useful enhancement, and it seems to align with the direction of the UX/UI audit discussed in #690. (The Figma file linked in the discussion contains the visual proposals in English).

Thanks for having looked at this, and your enthusiasm :-)

Before we can consider this PR, a few points need to be addressed. The PR introduces GPSLoggerConnectionListener extends Activity and makes DisplayTrackMap and TrackLogger inherit from it. I'm cautious about this: TrackLogger is already planned for migration to AppCompatActivity (as a continuation of #713), and the intermediate Activity subclass would complicate that migration.

Addressed now by making this an interface rather than a superclass.

Also, GPSLogger already handles INTENT_START_TRACKING and INTENT_STOP_TRACKING in its BroadcastReceiver. Using only the existing broadcasts would be more consistent with the current pattern in GPSLogger. It would also be great if the start/stop logic could work generally, not only from the map view.

Actually, my code does use these INTENT_START_TRACKING and INTENT_STOP_TRACKING intents sent to the broadcast receiver, just like TrackManager does. However, as far as starting goes, INTENT_START_TRACKING is not sent directly, but by starting the service. The service then sends to BroadcastReceiver. This is also how it happens in TrackManager.

However, you are right insofar that there are a couple of new data paths that were not there before. These go into the other direction, i.e. from logging component back to display:

1. DisplayTrackMap reads GPSLogger's isTracking to update the color of the button (red or green)
2. DisplayTrackMap informs TrackManager whether that the current track is no longer active, so that it can remove (or add back) the small orange clock icon.

I'm considering removing those paths, and replacing them with handlers in onResume that check the database for the active track (via dataHelper.getActiveTrackId()

While thinking of this, I noticed an inconsistency which already existed in the unmodified code: for stopping tracking, it's GPSLogger who updates the database via dataHelper.stopTracking(trackId). However, for starting it is the caller (who starts the service) which is doing it via activity.getContentResolver().update() to set the track's COL_ACTIVE to VAL_TRACK_ACTIVE. But even for stopping, the caller is calling dataHelper.stopTracking(trackId) too, "for good measure".

I'm really unsure what to do about this.

An alternative worth considering is reusing the existing gpsstatus_record.xml layout (or a simplified version) in DisplayTrackMap or adding a menu item.

I'm hesitating about this. Indeed, the goal of the feature is to make start/stopping the track more accessible, i.e. directly on the map screen. gpsstatus_record.xml is an element of the TrackLogger, i.e. the screen with the waypoint squares that comes just before the map. So, in order to use that element, you'd need to page back to that page, which I wanted to avoid.

The PR also lacks automated tests, which are important given that it touches the lifecycle of two Activities and the start/stop flow.

While attempting to set up this automated test, I noticed the inconsistency mentioned above, and now I'm hesitating whether to clean that up or not before writing the test. Also, eliminating those extra data paths, and replacing them with onResume() will simplify the test case.

To keep the discussion organized, could you open an issue describing the feature and the proposed approach first? That would give us room to agree on the UI changes and the communication mechanism with GPSLogger before we land the implementation. Thanks again for the contribution.

Done: #720