Skip to content

Conversation

Choromanski
Copy link

@Choromanski Choromanski commented Aug 2, 2025

User description

Instead of using central cords of city +/- buffer get the boundary box of the city +/- buffer


PR Type

Enhancement


Description

  • Replace city center coordinates with actual boundary boxes

  • Use bounding box coordinates from geocoding API response

  • Apply buffer to boundary extents instead of center point

  • Improve geographic accuracy for city-based tile downloads


Diagram Walkthrough

flowchart LR
  A["City Geocoding"] --> B["Extract Bounding Box"]
  B --> C["Apply Buffer to Bounds"]
  C --> D["Calculate Final Area"]
  E["Old: Center + Buffer"] -.-> F["New: Boundary + Buffer"]
Loading

File Walkthrough

Relevant files
Enhancement
meshtastic_tiles.py
Replace center coordinates with boundary boxes                     

meshtastic_tiles.py

  • Extract bounding box coordinates from geocoding API response
  • Replace center-based calculations with boundary-based calculations
  • Update coordinate handling in both single and multiple city workflows
  • Apply buffer to actual city boundaries instead of center points
+16/-10 

Copy link

qodo-merge-pro bot commented Aug 2, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Validation

The code assumes boundingbox array always has 4 elements when extracting coordinates. If the geocoding API returns incomplete or malformed bounding box data, this could cause IndexError exceptions.

'south': float(result['boundingbox'][0]),
'north': float(result['boundingbox'][1]),
'west': float(result['boundingbox'][2]),
'east': float(result['boundingbox'][3]),
Buffer Approximation

The buffer conversion uses a fixed approximation of 111km per degree, which is only accurate at the equator. At higher latitudes, longitude degrees represent shorter distances, making the buffer inaccurate.

buffer_deg = buffer_km / 111.0  # ~111km per degree

Copy link

qodo-merge-pro bot commented Aug 2, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add bounding box validation

Add validation to ensure the boundingbox array has at least 4 elements before
accessing indices. This prevents IndexError if the API returns incomplete
bounding box data.

meshtastic_tiles.py [56-59]

-'south': float(result['boundingbox'][0]),
-'north': float(result['boundingbox'][1]),
-'west': float(result['boundingbox'][2]),
-'east': float(result['boundingbox'][3]),
+bbox = result.get('boundingbox', [])
+if len(bbox) >= 4:
+    'south': float(bbox[0]),
+    'north': float(bbox[1]),
+    'west': float(bbox[2]),
+    'east': float(bbox[3]),
+else:
+    # Fallback to center point with small offset
+    lat, lon = float(result['lat']), float(result['lon'])
+    offset = 0.01  # ~1km
+    'south': lat - offset,
+    'north': lat + offset,
+    'west': lon - offset,
+    'east': lon + offset,

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential IndexError if the API returns incomplete boundingbox data and provides a sensible fallback, improving the code's robustness.

Medium
  • More

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.

1 participant