From ed13a4b4bac8ff5d6ac645143361f0f41b3d3d84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Boisselier?= Date: Sun, 16 Jun 2024 20:04:32 +0200 Subject: [PATCH 1/5] manually delete the Accelerometer object --- shaketune/commands/axes_map_calibration.py | 2 ++ shaketune/commands/axes_shaper_calibration.py | 2 ++ shaketune/commands/compare_belts_responses.py | 3 ++- shaketune/commands/create_vibrations_profile.py | 3 ++- shaketune/commands/excitate_axis_at_freq.py | 1 + 5 files changed, 9 insertions(+), 2 deletions(-) diff --git a/shaketune/commands/axes_map_calibration.py b/shaketune/commands/axes_map_calibration.py index f89d60b..a168d2f 100644 --- a/shaketune/commands/axes_map_calibration.py +++ b/shaketune/commands/axes_map_calibration.py @@ -82,6 +82,8 @@ def axes_map_calibration(gcmd, config, st_process: ShakeTuneProcess) -> None: toolhead.dwell(0.5) accelerometer.stop_measurement('axesmap_Z', append_time=True) + del accelerometer + # Re-enable the input shaper if it was active if input_shaper is not None: input_shaper.enable_shaping() diff --git a/shaketune/commands/axes_shaper_calibration.py b/shaketune/commands/axes_shaper_calibration.py index 8a2eb3d..d77d388 100644 --- a/shaketune/commands/axes_shaper_calibration.py +++ b/shaketune/commands/axes_shaper_calibration.py @@ -103,6 +103,8 @@ def axes_shaper_calibration(gcmd, config, st_process: ShakeTuneProcess) -> None: vibrate_axis(toolhead, gcode, config['direction'], min_freq, max_freq, hz_per_sec, accel_per_hz) accelerometer.stop_measurement(config['label'], append_time=True) + del accelerometer + # And finally generate the graph for each measured axis ConsoleOutput.print(f'{config["axis"].upper()} axis frequency profile generation...') ConsoleOutput.print('This may take some time (1-3min)') diff --git a/shaketune/commands/compare_belts_responses.py b/shaketune/commands/compare_belts_responses.py index 92375c7..31f4f52 100644 --- a/shaketune/commands/compare_belts_responses.py +++ b/shaketune/commands/compare_belts_responses.py @@ -58,7 +58,6 @@ def compare_belts_responses(gcmd, config, st_process: ShakeTuneProcess) -> None: raise gcmd.error( 'No suitable accelerometer found for measurement! Multi-accelerometer configurations are not supported for this macro.' ) - accelerometer = Accelerometer(printer.lookup_object(accel_chip)) # Move to the starting point test_points = res_tester.test.get_start_test_points() @@ -99,9 +98,11 @@ def compare_belts_responses(gcmd, config, st_process: ShakeTuneProcess) -> None: # Run the test for each axis for config in filtered_config: + accelerometer = Accelerometer(printer.lookup_object(accel_chip)) accelerometer.start_measurement() vibrate_axis(toolhead, gcode, config['direction'], min_freq, max_freq, hz_per_sec, accel_per_hz) accelerometer.stop_measurement(config['label'], append_time=True) + del accelerometer # Re-enable the input shaper if it was active if input_shaper is not None: diff --git a/shaketune/commands/create_vibrations_profile.py b/shaketune/commands/create_vibrations_profile.py index bcac671..95c68d5 100644 --- a/shaketune/commands/create_vibrations_profile.py +++ b/shaketune/commands/create_vibrations_profile.py @@ -90,7 +90,6 @@ def create_vibrations_profile(gcmd, config, st_process: ShakeTuneProcess) -> Non k_accelerometer = printer.lookup_object(current_accel_chip, None) if k_accelerometer is None: raise gcmd.error(f'Accelerometer [{current_accel_chip}] not found!') - accelerometer = Accelerometer(k_accelerometer) ConsoleOutput.print(f'Accelerometer chip used for this angle: [{current_accel_chip}]') # Sweep the speed range to record the vibrations at different speeds @@ -121,12 +120,14 @@ def create_vibrations_profile(gcmd, config, st_process: ShakeTuneProcess) -> Non movements = 2 # Back and forth movements to record the vibrations at constant speed in both direction + accelerometer = Accelerometer(k_accelerometer) accelerometer.start_measurement() for _ in range(movements): toolhead.move([mid_x + dX, mid_y + dY, z_height, E], curr_speed) toolhead.move([mid_x - dX, mid_y - dY, z_height, E], curr_speed) name = f'vib_an{curr_angle:.2f}sp{curr_speed:.2f}'.replace('.', '_') accelerometer.stop_measurement(name) + del accelerometer toolhead.dwell(0.3) toolhead.wait_moves() diff --git a/shaketune/commands/excitate_axis_at_freq.py b/shaketune/commands/excitate_axis_at_freq.py index d1ecd63..b9d6e0b 100644 --- a/shaketune/commands/excitate_axis_at_freq.py +++ b/shaketune/commands/excitate_axis_at_freq.py @@ -100,6 +100,7 @@ def excitate_axis_at_freq(gcmd, config, st_process: ShakeTuneProcess) -> None: # If the user wanted to create a graph, we stop the recording and generate it if create_graph: accelerometer.stop_measurement(f'staticfreq_{axis.upper()}', append_time=True) + del accelerometer creator = st_process.get_graph_creator() creator.configure(freq, duration, accel_per_hz) From dc49464adfc2dd5e6b3098d1926dd90fde60b3d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Boisselier?= Date: Sun, 16 Jun 2024 20:09:56 +0200 Subject: [PATCH 2/5] placing the scheduler into batch mode to drastically reduce S&T process priority --- shaketune/shaketune_process.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/shaketune/shaketune_process.py b/shaketune/shaketune_process.py index a6ff401..7b418a8 100644 --- a/shaketune/shaketune_process.py +++ b/shaketune/shaketune_process.py @@ -58,10 +58,12 @@ def _handle_timeout(self) -> None: os._exit(1) # Forcefully exit the process def _shaketune_process(self, graph_creator) -> None: - # Trying to reduce Shake&Tune process priority to avoid slowing down the main Klipper process - # as this could lead to random "Timer too close" errors when already running CANbus, etc... + # Reducing Shake&Tune process priority by putting the scheduler into batch mode with low priority. This in order to avoid + # slowing down the main Klipper process as this can lead to random "Timer too close" or "Move queue overflow" errors + # when also already running CANbus, neopixels and other consumming stuff in Klipper's main process. try: - os.nice(19) + param = os.sched_param(os.sched_get_priority_min(os.SCHED_BATCH)) + os.sched_setscheduler(0, os.SCHED_BATCH, param) except Exception: ConsoleOutput.print('Warning: failed reducing Shake&Tune process priority, continuing...') From e4a625587a39e73516d31741367c3592d600a01b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Boisselier?= Date: Sun, 16 Jun 2024 23:32:32 +0200 Subject: [PATCH 3/5] put the accelerometer file writes in a subprocess --- shaketune/commands/accelerometer.py | 30 +++++++++++++++---- shaketune/commands/axes_map_calibration.py | 2 +- shaketune/commands/axes_shaper_calibration.py | 2 +- shaketune/commands/compare_belts_responses.py | 5 ++-- .../commands/create_vibrations_profile.py | 5 ++-- shaketune/commands/excitate_axis_at_freq.py | 2 +- shaketune/shaketune_process.py | 6 ++-- 7 files changed, 36 insertions(+), 16 deletions(-) diff --git a/shaketune/commands/accelerometer.py b/shaketune/commands/accelerometer.py index a745199..3fbde12 100644 --- a/shaketune/commands/accelerometer.py +++ b/shaketune/commands/accelerometer.py @@ -9,15 +9,18 @@ # accelerometer measurements and write the data to a file in a blocking manner. +import os import time - -# from ..helpers.console_output import ConsoleOutput +from multiprocessing import Process, Queue class Accelerometer: def __init__(self, klipper_accelerometer): self._k_accelerometer = klipper_accelerometer + self._bg_client = None + self._write_queue = Queue() + self._write_processes = [] @staticmethod def find_axis_accelerometer(printer, axis: str = 'xy'): @@ -32,7 +35,6 @@ def find_axis_accelerometer(printer, axis: str = 'xy'): def start_measurement(self): if self._bg_client is None: self._bg_client = self._k_accelerometer.start_internal_client() - # ConsoleOutput.print('Accelerometer measurements started') else: raise ValueError('measurements already started!') @@ -54,12 +56,30 @@ def stop_measurement(self, name: str = None, append_time: bool = True): bg_client.finish_measurements() filename = f'/tmp/shaketune-{name}.csv' - self._write_to_file(bg_client, filename) - # ConsoleOutput.print(f'Accelerometer measurements stopped. Data written to {filename}') + self._queue_file_write(bg_client, filename) + + def _queue_file_write(self, bg_client, filename): + self._write_queue.put(filename) + write_proc = Process(target=self._write_to_file, args=(bg_client, filename)) + write_proc.daemon = True + write_proc.start() + self._write_processes.append(write_proc) def _write_to_file(self, bg_client, filename): + try: + os.nice(20) + except Exception: + pass with open(filename, 'w') as f: f.write('#time,accel_x,accel_y,accel_z\n') samples = bg_client.samples or bg_client.get_samples() for t, accel_x, accel_y, accel_z in samples: f.write(f'{t:.6f},{accel_x:.6f},{accel_y:.6f},{accel_z:.6f}\n') + self._write_queue.get() + + def wait_for_file_writes(self): + while not self._write_queue.empty(): + time.sleep(0.1) + for proc in self._write_processes: + proc.join() + self._write_processes = [] diff --git a/shaketune/commands/axes_map_calibration.py b/shaketune/commands/axes_map_calibration.py index a168d2f..0b17ee3 100644 --- a/shaketune/commands/axes_map_calibration.py +++ b/shaketune/commands/axes_map_calibration.py @@ -82,7 +82,7 @@ def axes_map_calibration(gcmd, config, st_process: ShakeTuneProcess) -> None: toolhead.dwell(0.5) accelerometer.stop_measurement('axesmap_Z', append_time=True) - del accelerometer + accelerometer.wait_for_file_writes() # Re-enable the input shaper if it was active if input_shaper is not None: diff --git a/shaketune/commands/axes_shaper_calibration.py b/shaketune/commands/axes_shaper_calibration.py index d77d388..6b919cf 100644 --- a/shaketune/commands/axes_shaper_calibration.py +++ b/shaketune/commands/axes_shaper_calibration.py @@ -103,7 +103,7 @@ def axes_shaper_calibration(gcmd, config, st_process: ShakeTuneProcess) -> None: vibrate_axis(toolhead, gcode, config['direction'], min_freq, max_freq, hz_per_sec, accel_per_hz) accelerometer.stop_measurement(config['label'], append_time=True) - del accelerometer + accelerometer.wait_for_file_writes() # And finally generate the graph for each measured axis ConsoleOutput.print(f'{config["axis"].upper()} axis frequency profile generation...') diff --git a/shaketune/commands/compare_belts_responses.py b/shaketune/commands/compare_belts_responses.py index 31f4f52..f2964f1 100644 --- a/shaketune/commands/compare_belts_responses.py +++ b/shaketune/commands/compare_belts_responses.py @@ -58,6 +58,7 @@ def compare_belts_responses(gcmd, config, st_process: ShakeTuneProcess) -> None: raise gcmd.error( 'No suitable accelerometer found for measurement! Multi-accelerometer configurations are not supported for this macro.' ) + accelerometer = Accelerometer(printer.lookup_object(accel_chip)) # Move to the starting point test_points = res_tester.test.get_start_test_points() @@ -98,11 +99,11 @@ def compare_belts_responses(gcmd, config, st_process: ShakeTuneProcess) -> None: # Run the test for each axis for config in filtered_config: - accelerometer = Accelerometer(printer.lookup_object(accel_chip)) accelerometer.start_measurement() vibrate_axis(toolhead, gcode, config['direction'], min_freq, max_freq, hz_per_sec, accel_per_hz) accelerometer.stop_measurement(config['label'], append_time=True) - del accelerometer + + accelerometer.wait_for_file_writes() # Re-enable the input shaper if it was active if input_shaper is not None: diff --git a/shaketune/commands/create_vibrations_profile.py b/shaketune/commands/create_vibrations_profile.py index 95c68d5..efdc54a 100644 --- a/shaketune/commands/create_vibrations_profile.py +++ b/shaketune/commands/create_vibrations_profile.py @@ -91,6 +91,7 @@ def create_vibrations_profile(gcmd, config, st_process: ShakeTuneProcess) -> Non if k_accelerometer is None: raise gcmd.error(f'Accelerometer [{current_accel_chip}] not found!') ConsoleOutput.print(f'Accelerometer chip used for this angle: [{current_accel_chip}]') + accelerometer = Accelerometer(k_accelerometer) # Sweep the speed range to record the vibrations at different speeds for curr_speed_sample in range(nb_speed_samples): @@ -120,18 +121,18 @@ def create_vibrations_profile(gcmd, config, st_process: ShakeTuneProcess) -> Non movements = 2 # Back and forth movements to record the vibrations at constant speed in both direction - accelerometer = Accelerometer(k_accelerometer) accelerometer.start_measurement() for _ in range(movements): toolhead.move([mid_x + dX, mid_y + dY, z_height, E], curr_speed) toolhead.move([mid_x - dX, mid_y - dY, z_height, E], curr_speed) name = f'vib_an{curr_angle:.2f}sp{curr_speed:.2f}'.replace('.', '_') accelerometer.stop_measurement(name) - del accelerometer toolhead.dwell(0.3) toolhead.wait_moves() + accelerometer.wait_for_file_writes() + # Restore the previous acceleration values gcode.run_script_from_command( f'SET_VELOCITY_LIMIT ACCEL={old_accel} MINIMUM_CRUISE_RATIO={old_mcr} SQUARE_CORNER_VELOCITY={old_sqv}' diff --git a/shaketune/commands/excitate_axis_at_freq.py b/shaketune/commands/excitate_axis_at_freq.py index b9d6e0b..c8aa1d6 100644 --- a/shaketune/commands/excitate_axis_at_freq.py +++ b/shaketune/commands/excitate_axis_at_freq.py @@ -100,7 +100,7 @@ def excitate_axis_at_freq(gcmd, config, st_process: ShakeTuneProcess) -> None: # If the user wanted to create a graph, we stop the recording and generate it if create_graph: accelerometer.stop_measurement(f'staticfreq_{axis.upper()}', append_time=True) - del accelerometer + accelerometer.wait_for_file_writes() creator = st_process.get_graph_creator() creator.configure(freq, duration, accel_per_hz) diff --git a/shaketune/shaketune_process.py b/shaketune/shaketune_process.py index 7b418a8..04a2d85 100644 --- a/shaketune/shaketune_process.py +++ b/shaketune/shaketune_process.py @@ -8,10 +8,10 @@ # vibration analysis processes in separate system processes. -import multiprocessing import os import threading import traceback +from multiprocessing import Process from typing import Optional from .helpers.console_output import ConsoleOutput @@ -31,9 +31,7 @@ def get_graph_creator(self): def run(self) -> None: # Start the target function in a new process (a thread is known to cause issues with Klipper and CANbus due to the GIL) - self._process = multiprocessing.Process( - target=self._shaketune_process_wrapper, args=(self.graph_creator, self._timeout) - ) + self._process = Process(target=self._shaketune_process_wrapper, args=(self.graph_creator, self._timeout)) self._process.start() def wait_for_completion(self) -> None: From 9b0ff2c00fdfc1ee8117a9d88eeede84758e59a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Boisselier?= Date: Mon, 17 Jun 2024 09:43:15 +0200 Subject: [PATCH 4/5] using reactor time instead of join to wait for S&T process to finish --- shaketune/shaketune.py | 35 +++++++++++++++++++++++++++++----- shaketune/shaketune_process.py | 23 +++++++++++++++------- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/shaketune/shaketune.py b/shaketune/shaketune.py index c710198..8a1641d 100644 --- a/shaketune/shaketune.py +++ b/shaketune/shaketune.py @@ -117,29 +117,54 @@ def __init__(self, config) -> None: def cmd_EXCITATE_AXIS_AT_FREQ(self, gcmd) -> None: ConsoleOutput.print(f'Shake&Tune version: {ShakeTuneConfig.get_git_version()}') static_freq_graph_creator = StaticGraphCreator(self._config) - st_process = ShakeTuneProcess(self._config, static_freq_graph_creator, self.timeout) + st_process = ShakeTuneProcess( + self._config, + self._printer.get_reactor(), + static_freq_graph_creator, + self.timeout, + ) excitate_axis_at_freq(gcmd, self._pconfig, st_process) def cmd_AXES_MAP_CALIBRATION(self, gcmd) -> None: ConsoleOutput.print(f'Shake&Tune version: {ShakeTuneConfig.get_git_version()}') axes_map_graph_creator = AxesMapGraphCreator(self._config) - st_process = ShakeTuneProcess(self._config, axes_map_graph_creator, self.timeout) + st_process = ShakeTuneProcess( + self._config, + self._printer.get_reactor(), + axes_map_graph_creator, + self.timeout, + ) axes_map_calibration(gcmd, self._pconfig, st_process) def cmd_COMPARE_BELTS_RESPONSES(self, gcmd) -> None: ConsoleOutput.print(f'Shake&Tune version: {ShakeTuneConfig.get_git_version()}') belt_graph_creator = BeltsGraphCreator(self._config) - st_process = ShakeTuneProcess(self._config, belt_graph_creator, self.timeout) + st_process = ShakeTuneProcess( + self._config, + self._printer.get_reactor(), + belt_graph_creator, + self.timeout, + ) compare_belts_responses(gcmd, self._pconfig, st_process) def cmd_AXES_SHAPER_CALIBRATION(self, gcmd) -> None: ConsoleOutput.print(f'Shake&Tune version: {ShakeTuneConfig.get_git_version()}') shaper_graph_creator = ShaperGraphCreator(self._config) - st_process = ShakeTuneProcess(self._config, shaper_graph_creator, self.timeout) + st_process = ShakeTuneProcess( + self._config, + self._printer.get_reactor(), + shaper_graph_creator, + self.timeout, + ) axes_shaper_calibration(gcmd, self._pconfig, st_process) def cmd_CREATE_VIBRATIONS_PROFILE(self, gcmd) -> None: ConsoleOutput.print(f'Shake&Tune version: {ShakeTuneConfig.get_git_version()}') vibration_profile_creator = VibrationsGraphCreator(self._config) - st_process = ShakeTuneProcess(self._config, vibration_profile_creator, self.timeout) + st_process = ShakeTuneProcess( + self._config, + self._printer.get_reactor(), + vibration_profile_creator, + self.timeout, + ) create_vibrations_profile(gcmd, self._pconfig, st_process) diff --git a/shaketune/shaketune_process.py b/shaketune/shaketune_process.py index 04a2d85..687ae8f 100644 --- a/shaketune/shaketune_process.py +++ b/shaketune/shaketune_process.py @@ -19,11 +19,11 @@ class ShakeTuneProcess: - def __init__(self, config: ShakeTuneConfig, graph_creator, timeout: Optional[float] = None) -> None: - self._config = config + def __init__(self, st_config: ShakeTuneConfig, reactor, graph_creator, timeout: Optional[float] = None) -> None: + self._config = st_config + self._reactor = reactor self.graph_creator = graph_creator self._timeout = timeout - self._process = None def get_graph_creator(self): @@ -35,16 +35,25 @@ def run(self) -> None: self._process.start() def wait_for_completion(self) -> None: - if self._process is not None: - self._process.join() + if self._process is None: + return # Nothing to wait for + eventtime = self._reactor.monotonic() + endtime = eventtime + self._timeout + complete = False + while eventtime < endtime: + eventtime = self._reactor.pause(eventtime + 0.05) + if not self._process.is_alive(): + complete = True + break + if not complete: + self._handle_timeout() # This function is a simple wrapper to start the Shake&Tune process. It's needed in order to get the timeout # as a Timer in a thread INSIDE the Shake&Tune child process to not interfere with the main Klipper process def _shaketune_process_wrapper(self, graph_creator, timeout) -> None: if timeout is not None: - timer = threading.Timer(timeout, self._handle_timeout) + timer = threading.Timer(timeout + 5, self._handle_timeout) # Add 5 seconds to the timeout for safety timer.start() - try: self._shaketune_process(graph_creator) finally: From 5ac8c9c1406b9bafdf8a353d80a450a8122c6554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Boisselier?= Date: Mon, 17 Jun 2024 19:39:11 +0200 Subject: [PATCH 5/5] added comment about the additional time for Timer timeout --- shaketune/shaketune_process.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/shaketune/shaketune_process.py b/shaketune/shaketune_process.py index 687ae8f..45012e8 100644 --- a/shaketune/shaketune_process.py +++ b/shaketune/shaketune_process.py @@ -52,7 +52,10 @@ def wait_for_completion(self) -> None: # as a Timer in a thread INSIDE the Shake&Tune child process to not interfere with the main Klipper process def _shaketune_process_wrapper(self, graph_creator, timeout) -> None: if timeout is not None: - timer = threading.Timer(timeout + 5, self._handle_timeout) # Add 5 seconds to the timeout for safety + # Add 5 seconds to the timeout for safety. The goal is to avoid the Timer to finish before the + # Shake&Tune process is done in case we call the wait_for_completion() function that uses Klipper's reactor. + timeout += 5 + timer = threading.Timer(timeout, self._handle_timeout) timer.start() try: self._shaketune_process(graph_creator)