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

Upload traces directly to Github. Corrections to PR361 #460

Draft
wants to merge 35 commits into
base: develop
Choose a base branch
from

Conversation

miltonials
Copy link

Hi @Binnette! I have checked PR#361 (WIP - New feature: Upload traces directly to Github) using the current state of the develop branch. The PR works fine, but I have found a problem when the trace size is too large, I have fixed this bug and a conflict in this PR.
I will appreciate if you have some suggestions to implement these fixes. If the new functionality is implemented, I may update the wiki to put the usage instructions for this new functionality.

Remark: the new functionality (pr#361) needs a classic gh token, instead of a fine-granuled token.

This PR is related to
#344
#361

Commits


Special thanks to @FR3DD221 for helping me find issues of the PR#361.

JeanMarcoRU and others added 30 commits September 14, 2022 16:46
…position fix is obtained

After the SDK update, UI buttons for marking waypoints remained disabled even after an accurate fix was acquired. This fix ensures that the UI buttons are enabled as soon as an accurate fix is confirmed, improving user experience in "hot start" situations.
… a temporary file path

- Modified `uploadTrackToGitHubAUX` to save the GPX file in Base64 to internal storage and pass only the file path in the Intent instead of the full content.
- Updated `GitHubUpload` to read the GPX file from the provided path, avoiding large Bundles.
- Added error handling for GPX file reading, displaying a message if reading fails.

This change prevents oversized Bundle errors and optimizes data transfer between activities.
Ensured broadcast intents are set with the package name in TagButtonOnClickListener.java, this was causing that the waypoints select meanwhile a track is recorded weren´t been saved.
@miltonials miltonials changed the base branch from master to develop November 21, 2024 21:09
@miltonials miltonials marked this pull request as ready for review November 21, 2024 21:15
@Binnette
Copy link
Collaborator

Hi @miltonials, Thank you for working on this PR.

I was wondering, if we really need a "Upload track to GitHub" option. We should encourage users to upload their track on OpenStreetMap directly. But the problem with OpenStreetMap is that it cannot store pictures and voice recording made with OSMTracker.

So my question is "does this PR also upload pictures and voice recordings to GitHub ?" If not this "feature" is not necessary as an upload on OpenStreetMap does a same plus have the advantage that the GPX track is also shared to OSM community.

I saw that this PR is using GitHub Token, it can create repo, create fork and so on. It seems a bit too "overkill" for me. And I am really not sure much people will use this feature. I would be more confident if more people show interest in this feature before merging it in OSMTracker.

So @miltonials maybe don't spend much more time as there are many others issues that would be more "interesting" to fix. I will review all issues and try to add priority tags to them.

Other questions/review :

  1. I tested the feature, after clicking "Upload to GitHub" button everything is in spanish, can you please provide english translation as well.
  2. In fact I see that the Spanish strings are hard coded in the layouts files and the on. So use @string instead on put them in strings.xml
  3. I see some commented out code (like //String GPXinBase64...) can you please remove all of them

@miltonials
Copy link
Author

miltonials commented Nov 23, 2024

Hi, thanks for the feedback @Binnette! I also agree with you that this implementation needs upload files like pictures, voice notes and all the information related with the trace. At the moment it just uploads the GPX file (similar to the "Share GPX" option). Since the "Export as GPX" functionality already exports all these files I think it can be reused to solve this and upload maybe a zip file.

I also wondered whether this is a need for users. Searching through different issues I saw that uploading tracking information to another server besides OSM is requested by different users. It was said in #85 that two functionalities were required.

  • Facilitate the download and installation of custom Layouts.
  • Upload traces to different servers, in addition to OSM.

In this same issue are other comments that agree with both functionalities. From what I read in #344, @JeanMarcoRU propose upload to GitHub to solve the second need and at the same time the users would have a shared repo and #361 (where the implementation is) was added to the next milestone.

From my perspective, this feature could be especially useful for teams collaborating on mapping projects. For example, mapping different national parks of a country, each team can go to different location and all of them could share their traces directly from OSMTracker. Similarly, it would be useful when teams are collaborating in the same location but they are collecting traces along different paths, with the goal of merging them using tools like QGIS, they would use the repository like a "real time" backup or shared folder, without leaving OSMTracker.

What do you think about these ideas, @Binnette? I'm open to iterating on this to better align with the community's priorities.


At the moment I will be working on the translations to English and the other observations in Other questions/review .

@miltonials miltonials marked this pull request as draft November 25, 2024 20:39
@Binnette Binnette changed the title Corrections to PR361 Upload traces directly to Github. Corrections to PR361 Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants