Yet another attempt to solve TTC errors...

[Frix-x]

Summary by Sourcery

Refactor the accelerometer data saving mechanism to use a dedicated writer process and queue. This prevents blocking I/O operations and improves data integrity. Update the data format to JSON lines for improved handling and parsing. Modify the shaketune_process.py to accept filenames as input and load data accordingly. Update graph creation to use the new data loading mechanism and save graphs with consistent naming. Fix various bugs related to data handling and analysis, and improve error handling and reporting.

Bug Fixes:

• Resolve "Timer too close" errors encountered during accelerometer data saving by implementing a dedicated writer process and queue to handle disk I/O operations asynchronously, preventing Klipper's main thread from being blocked during data saving, and ensuring measurements are written to disk before starting a new one, thus avoiding timing conflicts and potential data loss or corruption.
• Fix potential race conditions and data corruption issues that could occur when saving accelerometer data to disk by using a dedicated writer process and queue, ensuring that only one process writes to the file at a time, and preventing concurrent access and potential data corruption or inconsistencies.
• Fix potential issues with accelerometer data retrieval and graph generation by ensuring the Klipper's main thread is not blocked during data saving, allowing for timely retrieval of samples and preventing potential delays or errors in graph creation.
• Fix potential issues with data loss or corruption when assembling measurement chunks by ensuring proper synchronization and data integrity during the merging process, preventing data loss or corruption due to concurrent access or incomplete writes.
• Fix potential issues with data loading and processing by ensuring that the correct number of measurements is provided for each analysis tool, preventing errors and ensuring accurate results.
• Fix potential issues with incorrect measurement names by validating the provided names and ensuring they adhere to the required format, preventing errors and ensuring data consistency.

Enhancements:

• Improve the efficiency and performance of accelerometer data saving by using a dedicated writer process and queue, allowing asynchronous disk I/O operations and preventing Klipper's main thread from being blocked, thus improving overall responsiveness and performance.
• Improve data integrity and reliability by using a dedicated writer process and queue, ensuring that measurements are written to disk in a consistent and synchronized manner, preventing data loss or corruption due to concurrent access or incomplete writes.
• Simplify the data saving and loading process by using JSON serialization and a line-based format, improving code readability and maintainability, and reducing the complexity of data handling operations.
• Improve error handling and reporting during data saving and loading operations, providing more informative error messages and facilitating debugging and troubleshooting.
• Improve the accuracy and reliability of data analysis by ensuring that the correct number of measurements is provided for each tool, preventing errors and ensuring accurate results.
• Improve the overall user experience by providing clearer error messages and instructions, and ensuring data consistency and reliability.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request refactors the way accelerometer data is saved and processed in ShakeTune. It introduces a dedicated writer process to handle disk writes, which prevents blocking I/O and improves performance. The changes also simplify the graph creation process and ensure that data is saved correctly.

Sequence diagram for saving accelerometer data

sequenceDiagram
  participant ST as ShakeTune
  participant MM as MeasurementsManager
  participant WP as WriterProcess
  participant Queue as WriterQueue
  participant Disk

  ST->>MM: add_measurement(name, samples)
  alt len(measurements) > chunk_size
    MM->>Queue: put(measurement)
    activate WP
    Queue->>WP: meas
    WP->>Disk: write(meas)
    deactivate WP
  end
  ST->>MM: save_stdata(filename)
  MM->>Queue: put(STOP_SENTINEL)
  WP->>Disk: flush()
  Disk-->>WP: done
  WP-->>MM: done
  MM-->>ST: done

Loading

Updated class diagram for MeasurementsManager

classDiagram
    class MeasurementsManager {
        -chunk_size: int
        -k_reactor
        -measurements: List~Measurement~
        -temp_file: Path
        -writer_queue: Queue
        -is_writing: Value
        -writer_process: Optional~Process~
        +__init__(chunk_size: int, k_reactor=None)
        +clear_measurements(keep_last: bool = False)
        +append_samples_to_current_measurement(additional_samples: SamplesList)
        +add_measurement(name: str, samples: SamplesList = None, timeout: float = 30)
        -_writer_loop(output_file: Path, write_queue: Queue, is_writing: Value)
        -_flush_chunk()
        +save_stdata(filename: Path, timeout: int = 30)
        +get_measurements() : List~Measurement~
        +load_from_stdata(filename: Path) : List~Measurement~
        +load_from_csvs(klipper_CSVs: List~Path~) : List~Measurement~
        +__del__()
    }
    class Measurement {
        name: str
        samples: List~Sample~
    }
    Measurement -- MeasurementsManager: contains
    note for MeasurementsManager "Manages accelerometer measurements, including writing to disk using a dedicated process."

Loading

File-Level Changes

Change	Details	Files
Replaced the previous chunk-based file saving mechanism with a single temporary file and a dedicated writer process to handle disk writes.	Introduced a dedicated writer process with a queue for managing write operations. Utilized a single temporary file for storing accelerometer data. Implemented JSON serialization for writing measurement objects to disk. Used Zstandard compression for the data written to disk. Added a sentinel value to signal the writer process to stop. Implemented a timeout mechanism to prevent indefinite waiting for the writer process. Removed the chunk-based file saving mechanism. Removed the temporary directory creation and cleanup logic.	shaketune/helpers/accelerometer.py
Modified the ShakeTuneProcess class to accept a list of filenames instead of a MeasurementsManager object.	Updated the run method to accept a filename or a list of filenames. Modified the _shaketune_process method to load measurements from the provided file(s). Removed the measurements_manager parameter from the _shaketune_process method. Added file existence checks and suffix validation. Instantiated the MeasurementsManager inside the _shaketune_process method.	shaketune/shaketune_process.py
Updated the graph creators to save directly to the final file path and removed the intermediate .stdata file saving.	Added a define_output_target method to set the output file path. Modified the _save_figure method to save the figure to the defined output path. Removed the measurements_manager parameter from the _save_figure method. Removed the logic for saving the raw data to a separate .stdata file. Added a get_folder method to retrieve the output folder. Removed the override_output_target method. Removed the _graph_date attribute.	shaketune/graph_creators/graph_creator.py
Modified the commands to use the new data saving and graph creation flow.	Instantiated the MeasurementsManager with the Klipper reactor. Removed calls to accelerometer.wait_for_samples(). Removed calls to measurements_manager.wait_for_data_transfers(). Added logic to define the output target for the graph creator. Called measurements_manager.save_stdata() to save the data to disk. Passed the filename to st_process.run().	shaketune/commands/axes_shaper_calibration.py shaketune/commands/compare_belts_responses.py shaketune/commands/create_vibrations_profile.py shaketune/commands/axes_map_calibration.py shaketune/commands/excitate_axis_at_freq.py
Updated the graph computations to handle cases where there are not enough measurements.	Added a check to ensure that there are 3 measurements available before computing the axes map. Added a check to ensure that there are 2 measurements available before computing the belts graph.	shaketune/graph_creators/axes_map_graph_creator.py shaketune/graph_creators/belts_graph_creator.py
Updated the CLI to use the new output target definition.	Called graph_creator.define_output_target() instead of graph_creator.override_output_target().	shaketune/cli.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!
• Generate a plan of action for an issue: Comment @sourcery-ai plan on
  an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @Frix-x - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider adding a method to MeasurementsManager to check if the writer process is running.
• The __del__ method in MeasurementsManager might not always be called, so consider a more explicit cleanup mechanism.

Here's what I looked at during the review

• 🟡 General issues: 4 issues found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 1 issue found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}