Skip to content

feat: Several changes to libosmscout - #1760

Merged
Framstag merged 2 commits into
masterfrom
updates-to-general-libosmscout-code
Aug 21, 2026
Merged

feat: Several changes to libosmscout#1760
Framstag merged 2 commits into
masterfrom
updates-to-general-libosmscout-code

Conversation

@Framstag

@Framstag Framstag commented Aug 13, 2026

Copy link
Copy Markdown
Owner
  • KI infused change in context of Java-based client development in context of...
  • basemap support
  • Dynamic map loading/removing in client
  • routing implementation, improvement
  • Error handling

@Framstag
feat: Several changes to libosmscout
351ec5a 
- KI infused change in context of Java-based client development in context of...
- basemap support
- Dynamic map loading/removing in client
- routing implementation, improvement
- Error handling
@Framstag
Framstag requested a review from Karry August 13, 2026 18:30
@Framstag

Framstag commented Aug 14, 2026

Copy link
Copy Markdown
Owner Author

Hi @Karry, please check the changes if they are OK for you

@Framstag Framstag mentioned this pull request Aug 15, 2026
Open
Karry
Karry approved these changes Aug 20, 2026
View reviewed changes

@Karry Karry left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just few nitpicks :-)

Comment thread libosmscout-client/src/osmscoutclient/DBThread.cpp Outdated
Async<bool>([this](const Breaker& /*breaker*/) {
WriteLock locker(latch);

if (basemapDatabase) {

@Karry Karry Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines below are duplicate, I would love to have them in separate private method...

Comment thread libosmscout/src/osmscout/navigation/SpeedAgent.cpp

if (lastPosition &&
(gpsUpdateMsg->timestamp-lastPosition.time) >= seconds(1)){
// --- Prefer GPS-reported speed over position-difference computation ---

@Karry Karry Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, with my experience, speed reported by the device GPS module is nonsense usually. This is the reason why it is computed as average movement in few seconds... I will test this change. We may introduce some configuration option what speed value to prefer...

@Framstag Framstag Aug 21, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this part but note, that I currently also have effect with GPS signal handling, so I'm also sure that like GPS signal handling will change...

Comment thread libosmscout-client/src/osmscoutclient/MapManager.cpp
}),
databaseLookupDirs.end());
}
}

@Karry Karry Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not be LookupDatabases(); called here?

@Framstag
refactor: Extract DBThread::LoadBasemap, rescan on RemoveLookupDirectory
357c7be 
- Move duplicated basemap close/open logic from OnDatabaseListChanged and
  ReloadBasemap into private DBThread::LoadBasemap() method
- MapManager::RemoveLookupDirectory now triggers LookupDatabases() so maps
  from removed directory disappear from database list and
  databaseListChanged signal is emitted
- Add test verifying RemoveLookupDirectory rescans and emits signal
@Framstag

Framstag commented Aug 21, 2026

Copy link
Copy Markdown
Owner Author

Feedback form AI:

Done. All three comments handled.

Summary

Comment 1 — DBThread dedup (libosmscout-client/src/osmscoutclient/DBThread.cpp, DBThread.h)

  • Extracted basemap close+open logic into private DBThread::LoadBasemap()
  • OnDatabaseListChanged and ReloadBasemap now both call it
  • Preserved ordering: LoadBasemap() still called after mmap-quota computation in OnDatabaseListChanged (32-bit
    mmap flags must apply before open)
  • Unified log message ("Basemap loaded from...")

Comment 2 — SpeedAgent — ignored per instruction.

Comment 3 — MapManager (MapManager.cpp, MapManager.h)

  • Analysis: RemoveLookupDirectory removed dir from databaseLookupDirs but never re-scanned. Result:
    databaseDirectories still held maps from removed dir, databaseListChanged never emitted → DBThread kept removed
    databases open. Karry right.
  • Fix: added LookupDatabases() call, mirroring AddLookupDirectory
  • Header doc updated

Test (Tests/src/MapDownloadServiceTest.cpp)

  • New MapManager::RemoveLookupDirectory triggers database rescan:
    • Creates valid map dir (all 18 mandatory files)
    • AddLookupDirectory → map visible in GetDatabaseDirectories()
    • RemoveLookupDirectory → map gone, lookup dirs empty, databaseListChanged emitted with empty list
    • Polls async worker with deadline

Verification

  • Full build clean
  • 68/68 non-PerformanceTest tests pass (PerformanceTest failures pre-existing — UI lib leak on exit, documented in
    AGENTS.md)
  • New test: 12 test cases, 77 assertions, all pass

Changes uncommitted on branch updates-to-general-libosmscout-code. Want commit?

@sonarqubecloud

sonarqubecloud Bot commented Aug 21, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
9.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@Framstag
Framstag merged commit f302f3c into master Aug 21, 2026
20 of 21 checks passed
@Framstag
Framstag deleted the updates-to-general-libosmscout-code branch August 21, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Karry Karry Karry approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Framstag @Karry