Skip to content

Add proper cleanup and error handling #5

@luandro

Description

@luandro

Problem

The current implementation has two error handling issues:

1. No Cleanup on Error (Lines 89-91)

try:
    # ... generation code ...
except Exception as e:
    self.log(f"Error generating SMP file: {str(e)}", Qgis.Critical)
    raise  # temp_dir is never removed!

If an exception occurs during SMP generation, the temporary directory is never cleaned up, leaving files on disk.

2. Progress Reporting Resets Per Zoom Level (Lines 250-253)

# Update progress
if self.feedback:
    progress = (x * num_tiles_y + y) / (num_tiles_x * num_tiles_y) * 100
    self.feedback.setProgress(progress)

Progress calculation is per-zoom-level, so it resets to 0% at each new zoom level instead of showing overall progress.

Solutions

Fix 1: Add Cleanup Handler

def generate_smp_from_canvas(self, extent, min_zoom, max_zoom, output_path):
    self.log(f"Generating SMP file with zoom levels {min_zoom}-{max_zoom}")
    self.log(f"Extent: {extent.asWktPolygon()}")

    # Create a temporary directory for the SMP contents
    temp_dir = tempfile.mkdtemp()
    self.log(f"Using temporary directory: {temp_dir}")

    try:
        # Get the current project
        project = QgsProject.instance()

        # ... rest of generation code ...

        self.log(f"SMP file generated successfully: {output_path}")
        return output_path

    except Exception as e:
        self.log(f"Error generating SMP file: {str(e)}", Qgis.Critical)
        raise
    finally:
        # Always clean up temporary directory
        import shutil
        if os.path.exists(temp_dir):
            shutil.rmtree(temp_dir)
            self.log(f"Cleaned up temporary directory: {temp_dir}")

Fix 2: Calculate Overall Progress

def _generate_tiles_from_canvas(self, extent, min_zoom, max_zoom, tiles_dir):
    self.log("Generating tiles from map canvas...")

    # ... setup code ...

    # Calculate total tiles across all zoom levels
    total_tiles = 0
    tiles_by_zoom = []
    for zoom in range(min_zoom, max_zoom + 1):
        min_x, max_x, min_y, max_y = self._calculate_tiles_at_zoom(extent, zoom)
        num_tiles = (max_x - min_x + 1) * (max_y - min_y + 1)
        tiles_by_zoom.append((zoom, min_x, max_x, min_y, max_y, num_tiles))
        total_tiles += num_tiles

    self.log(f"Total tiles to generate: {total_tiles}")

    # Generate tiles with cumulative progress
    tiles_completed = 0
    for zoom, min_x, max_x, min_y, max_y, num_tiles in tiles_by_zoom:
        zoom_dir = os.path.join(tiles_dir, str(zoom))
        os.makedirs(zoom_dir, exist_ok=True)

        self.log(f"Zoom level {zoom}: {num_tiles} tiles")

        for x in range(min_x, max_x + 1):
            x_dir = os.path.join(zoom_dir, str(x))
            os.makedirs(x_dir, exist_ok=True)

            for y in range(min_y, max_y + 1):
                # ... render tile ...

                tiles_completed += 1
                
                # Update overall progress
                if self.feedback:
                    progress = (tiles_completed / total_tiles) * 100
                    self.feedback.setProgress(progress)

    self.log(f"Generated {tiles_completed} tiles from map canvas")

Impact

Low-Medium:

  • Cleanup issue causes disk space waste and potential file system clutter
  • Progress reporting issue creates poor user experience (appears stuck)
  • Both are quality-of-life improvements, not functionality blockers

Related Files

  • comapeo_smp_generator.py: Lines 47-91, 186-255

Additional Improvements

Consider adding:

  • Validation that output path is writable before starting
  • Disk space check before generation
  • Better error messages for common failure cases (permissions, disk space, invalid extent)

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions