-
-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LDO Nitehawk 36 #67
base: development
Are you sure you want to change the base?
LDO Nitehawk 36 #67
Conversation
Everything included. As soon as I get the hosted mage I will update wiring diagrams
change .bin to .u2f and hasBoot0Jumper to false
Everything tested on NW36. First flash has to be done via "Flash unidentified board". After that the auto flash works correctly. Correction on compile script for SB2209 with rp2040
WalkthroughThis pull request updates firmware build behavior and board configurations. The BTT board script now copies the firmware as Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as make-and-flash-mcu.sh
participant C as compile.sh
participant F as flash.sh
participant FP as flash-path.sh
U->>M: Execute make-and-flash-mcu.sh
M->>C: Invoke compile.sh (perform root check, compile firmware)
C-->>M: Return compilation result
M->>F: Invoke flash.sh (perform root check, determine flash path)
F->>FP: Execute flash-path.sh with MCU parameter
FP-->>F: Return flashing result
F-->>M: Flash process complete
M-->>U: Process complete
sequenceDiagram
participant D as USB Device
participant Udev as udev System
participant S as klipper-mcu-added.sh
D->>Udev: Device addition (idProduct: 614e, idVendor: 1d50, serial: ldo-nitehawk-36)
Udev->>Udev: Create symlink (/dev/ldo-nitehawk-36)
Udev->>S: Trigger klipper-mcu-added.sh
S-->>Udev: Script execution complete
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (3)
configuration/boards/ldo-nitehawk-36/98-ldo-nitehawk-36.rules (1)
4-4
: Add action for device removal.The remove action only checks for device link but doesn't perform any cleanup. Consider adding appropriate cleanup actions.
-ACTION=="remove", ENV{DEVLINKS}=="/dev/ldo-nitehawk-36" +ACTION=="remove", ENV{DEVLINKS}=="/dev/ldo-nitehawk-36", RUN+="/home/pi/printer_data/config/RatOS/scripts/klipper-mcu-removed.sh"configuration/boards/ldo-nitehawk-36/toolboard-config.cfg (1)
16-16
: Document null pin assignments.The BLTouch pins are set to null without explanation. Consider adding a comment explaining why these are not used.
- bltouch_sensor_pin=null, bltouch_control_pin=null, + # BLTouch is not supported on this board + bltouch_sensor_pin=null, bltouch_control_pin=null,configuration/boards/ldo-nitehawk-36/firmware.config (1)
40-43
: Remove unused CAN bus pin configurations.CAN bus is disabled (
CONFIG_RPXXXX_CANBUS is not set
) but pin configurations are present. This could lead to confusion.Remove the unused configurations:
-CONFIG_RPXXXX_CANBUS_GPIO_RX=4 -CONFIG_RPXXXX_CANBUS_GPIO_TX=5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
configuration/boards/ldo-nitehawk-36/dfubooting.png
is excluded by!**/*.png
configuration/boards/ldo-nitehawk-36/manual.pdf
is excluded by!**/*.pdf
configuration/boards/ldo-nitehawk-36/wiring.drawio.svg
is excluded by!**/*.svg
📒 Files selected for processing (8)
configuration/boards/btt-sb-2209-10-rp2040/compile.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/98-ldo-nitehawk-36.rules
(1 hunks)configuration/boards/ldo-nitehawk-36/board-definition.json
(1 hunks)configuration/boards/ldo-nitehawk-36/compile.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/firmware.config
(1 hunks)configuration/boards/ldo-nitehawk-36/flash.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/make-and-flash-mcu.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/toolboard-config.cfg
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint, Typecheck and Tests
🔇 Additional comments (3)
configuration/boards/btt-sb-2209-10-rp2040/compile.sh (1)
17-17
:❓ Verification inconclusive
Verify firmware file extension consistency.
The source file is
klipper.uf2
and the destination isfirmware-btt-sb-2209-10-rp.uf2
, which is correct. However, please ensure that the RP2040 bootloader expects the .uf2 format.
🏁 Script executed:
#!/bin/bash # Description: Check if klipper supports .uf2 format for RP2040 rg -A 5 "\.uf2" /home/pi/klipper/scriptsLength of output: 107
Ensure RP2040 bootloader supports UF2.
The change on line 17 in
configuration/boards/btt-sb-2209-10-rp2040/compile.sh
correctly updates the firmware file extension to.uf2
, aligning with the PR’s aim to update the firmware build behavior. However, the automated grep search in/home/pi/klipper/scripts
resulted in a “No such file or directory” error, so we couldn’t automatically confirm UF2 references. Please manually verify from the RP2040 bootloader documentation or related configuration files that the bootloader indeed expects the.uf2
format.configuration/boards/ldo-nitehawk-36/board-definition.json (1)
31-37
:❓ Verification inconclusive
Verify ADXL345 SPI pin assignments.
Please verify that the GPIO pins assigned for ADXL345 SPI communication match the hardware specifications:
- CS: gpio27
- SCLK: gpio18
- MOSI: gpio20
- MISO: gpio19
🌐 Web query:
What are the recommended GPIO pins for ADXL345 SPI communication on RP2040?
💡 Result:
The recommended GPIO pins for ADXL345 SPI communication on RP2040-based boards like Raspberry Pi Pico are configured through Klipper's flexible bus system. Here's the breakdown:
Primary SPI Configuration
For SPI0 connections using standard pin assignments:
ADXL345 Pin RP2040 Pin Function CS GPIO1 Chip Select SDO GPIO0 MISO (SPI RX) SDA GPIO3 MOSI (SPI TX) SCL GPIO2 SCK (Clock) This configuration is explicitly recommended in Klipper documentation[4][7] and matches common implementations using
spi0a
bus designation[1].Alternative Configurations
Multiple SPI buses can be utilized through pin remapping:
Second ADXL345 example using
spi1a
:[adxl345 bed] cs_pin: pico_adxl345:gpio9 spi_bus: spi1a
Requires using GPIO8-11 for SPI1 signals[1][6]
Experimental
spi0c
configuration:cs_pin: pico:gpio17 spi_bus: spi0cDemonstrates RP2040's flexible peripheral pin mapping[1]
Key Considerations
- Power connections must use 3.3V and GND pins (e.g., Pin 36/38 on Pico)[4][7]
- Always verify wiring with
ACCELEROMETER_QUERY
- invalid ID errors indicate connection issues[1]- SPI bus selection depends on physical wiring:
spi0a
: Default CS at GPIO1spi1a
: Requires separate CS pin like GPIO9[1][6]- Axes mapping often requires adjustment like
axes_map: -z,x,-y
for proper orientation[1]The configuration ultimately depends on your physical wiring layout and required number of sensors, but the primary recommendation uses SPI0 with GPIO0-3 plus a dedicated CS pin like GPIO1[4][7][1].
Citations:
- 1: https://klipper.discourse.group/t/raspberry-pi-pico-adxl345-portable-resonance-measurement/1757
- 2: https://mischianti.org/waveshare-rp2040-zero-high-resolution-pinout-and-specs/
- 3: https://forum.arduino.cc/t/adxl345-through-spi/155580
- 4: https://www.klipper3d.org/Measuring_Resonances.html
- 5: https://forums.raspberrypi.com/viewtopic.php?t=98632
- 6: https://forum.arduino.cc/t/rp2040-spi-0-four-sets-of-pins-named-spi-0-which-one-is-the-default-how-to-change-it/1222201
- 7: https://www.klipper3d.org/Measuring_Resonances.html?h=adxl
- 8: https://www.youtube.com/watch?v=W_VHbT_tsZw
- 9: https://forums.adafruit.com/viewtopic.php?t=85618
- 10: https://shillehtek.com/blogs/shillehtek-product-manuals/adxl345-accelerometer-3dof-raspberry-pi-arduino-esp32-i2c-accelerometer-klipper-tuning
Action: Validate ADXL345 SPI Pin Configuration
The configuration in
configuration/boards/ldo-nitehawk-36/board-definition.json
(lines 31–37) currently assigns:
- CS: gpio27
- SCLK: gpio18
- MOSI: gpio20
- MISO: gpio19
Based on the RP2040 recommendations (using SPI0 for a primary configuration), the typical pin assignments are:
- CS: GPIO1
- SCLK: GPIO2
- MOSI: GPIO3
- MISO: GPIO0
Please double-check that the current non-standard (or software) SPI configuration for the ADXL345 is intentional for your board’s wiring. If not, update the pin assignments accordingly.
configuration/boards/ldo-nitehawk-36/firmware.config (1)
69-69
:❓ Verification inconclusive
Verify initial pin state for safety.
The configuration sets
gpio8
to low on startup. Please verify that this is the intended behavior and won't cause any hardware issues.
🏁 Script executed:
#!/bin/bash # Description: Search for gpio8 usage in configuration files rg "gpio8" /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/Length of output: 168
Review Update: Verify Correct Pin Behavior and File Path
The automated search did not locate the file because the specified path (
/home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/
) isn’t correct. Please verify manually that the configuration inconfiguration/boards/ldo-nitehawk-36/firmware.config
—specifically the lineCONFIG_INITIAL_PINS="!gpio8"
—indeed setsgpio8
low on startup as intended without causing any hardware issues. Also, confirm that this file is being picked up in the build/runtime process and that no unintended side effects occur.
@@ -0,0 +1,4 @@ | |||
# LDO Nitehawk 36 | |||
SUBSYSTEMS=="usb", ATTRS{idProduct}=="614e", ATTRS{idVendor}=="1d50", ATTRS{serial}=="ldo-nitehawk-36", ACTION=="add", SYMLINK+="ldo-nitehawk-36", RUN+="/home/pi/printer_data/config/RatOS/scripts/klipper-mcu-added.sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the hardcoded script path.
The script path /home/pi/printer_data/config/RatOS/scripts/klipper-mcu-added.sh
assumes a specific directory structure. Consider making this path configurable or using environment variables.
🏁 Script executed:
#!/bin/bash
# Check if the script exists at the hardcoded path
if [ ! -f "/home/pi/printer_data/config/RatOS/scripts/klipper-mcu-added.sh" ]; then
echo "Warning: klipper-mcu-added.sh not found at expected path"
fi
Length of output: 203
Hardcoded Script Path Issue – Action Required
The verification revealed that the file /home/pi/printer_data/config/RatOS/scripts/klipper-mcu-added.sh
does not exist at the hardcoded location. To ensure reliability across different setups, please consider one of the following:
- Make the script path configurable (e.g., via a configuration file or environment variable)
- Ensure the script is correctly deployed at the expected location if retaining the hardcoded path
spi_software_sclk_pin: toolboard: gpio18 | ||
spi_software_mosi_pin: toolboard: gpio20 | ||
spi_software_miso_pin: toolboard: gpio19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix syntax errors in ADXL345 pin definitions.
There are spaces after the colons in the pin definitions which will cause configuration errors.
-spi_software_sclk_pin: toolboard: gpio18
-spi_software_mosi_pin: toolboard: gpio20
-spi_software_miso_pin: toolboard: gpio19
+spi_software_sclk_pin: toolboard:gpio18
+spi_software_mosi_pin: toolboard:gpio20
+spi_software_miso_pin: toolboard:gpio19
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
spi_software_sclk_pin: toolboard: gpio18 | |
spi_software_mosi_pin: toolboard: gpio20 | |
spi_software_miso_pin: toolboard: gpio19 | |
spi_software_sclk_pin: toolboard:gpio18 | |
spi_software_mosi_pin: toolboard:gpio20 | |
spi_software_miso_pin: toolboard:gpio19 |
"$SCRIPT_DIR"/compile.sh | ||
"$SCRIPT_DIR"/flash.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for script execution.
The script should check the return status of compile.sh and flash.sh and provide appropriate feedback.
-"$SCRIPT_DIR"/compile.sh
-"$SCRIPT_DIR"/flash.sh
+"$SCRIPT_DIR"/compile.sh
+if [ $? -ne 0 ]; then
+ echo "ERROR: Compilation failed"
+ exit 1
+fi
+
+"$SCRIPT_DIR"/flash.sh
+if [ $? -ne 0 ]; then
+ echo "ERROR: Flashing failed"
+ exit 1
+fi
+
+echo "Successfully compiled and flashed firmware"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"$SCRIPT_DIR"/compile.sh | |
"$SCRIPT_DIR"/flash.sh | |
"$SCRIPT_DIR"/compile.sh | |
if [ $? -ne 0 ]; then | |
echo "ERROR: Compilation failed" | |
exit 1 | |
fi | |
"$SCRIPT_DIR"/flash.sh | |
if [ $? -ne 0 ]; then | |
echo "ERROR: Flashing failed" | |
exit 1 | |
fi | |
echo "Successfully compiled and flashed firmware" |
FLASH_SCRIPT=$(realpath "$SCRIPT_DIR/../../scripts/flash-path.sh") | ||
$FLASH_SCRIPT $MCU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation for flash script and device.
The script should verify that both the flash script and device exist before proceeding.
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
FLASH_SCRIPT=$(realpath "$SCRIPT_DIR/../../scripts/flash-path.sh")
+
+if [ ! -f "$FLASH_SCRIPT" ]; then
+ echo "ERROR: Flash script not found at $FLASH_SCRIPT"
+ exit 1
+fi
+
+if [ ! -e "$MCU" ]; then
+ echo "ERROR: Device $MCU not found"
+ exit 1
+fi
+
$FLASH_SCRIPT $MCU
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
FLASH_SCRIPT=$(realpath "$SCRIPT_DIR/../../scripts/flash-path.sh") | |
$FLASH_SCRIPT $MCU | |
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | |
FLASH_SCRIPT=$(realpath "$SCRIPT_DIR/../../scripts/flash-path.sh") | |
if [ ! -f "$FLASH_SCRIPT" ]; then | |
echo "ERROR: Flash script not found at $FLASH_SCRIPT" | |
exit 1 | |
fi | |
if [ ! -e "$MCU" ]; then | |
echo "ERROR: Device $MCU not found" | |
exit 1 | |
fi | |
$FLASH_SCRIPT $MCU |
mkdir /home/pi/printer_data/config/firmware_binaries | ||
chown pi:pi /home/pi/printer_data/config/firmware_binaries | ||
fi | ||
cp -f /home/pi/klipper/out/klipper.bin /home/pi/printer_data/config/firmware_binaries/firmware-ldo-nitehawk-36.uf2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix firmware file extension mismatch.
The source file is klipper.bin
but the destination uses .uf2
extension. This mismatch could cause issues with the bootloader.
Apply this diff to fix the extension:
-cp -f /home/pi/klipper/out/klipper.bin /home/pi/printer_data/config/firmware_binaries/firmware-ldo-nitehawk-36.uf2
+cp -f /home/pi/klipper/out/klipper.bin /home/pi/printer_data/config/firmware_binaries/firmware-ldo-nitehawk-36.bin
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
cp -f /home/pi/klipper/out/klipper.bin /home/pi/printer_data/config/firmware_binaries/firmware-ldo-nitehawk-36.uf2 | |
cp -f /home/pi/klipper/out/klipper.bin /home/pi/printer_data/config/firmware_binaries/firmware-ldo-nitehawk-36.bin |
"name": "Nitehawk 36", | ||
"manufacturer": "LDO", | ||
"id": "ldo-nitehawk-36", | ||
"firmwareBinaryName": "firmware-ldo-nitehawk-36.bin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure firmware binary name consistency across files.
The firmwareBinaryName
uses .bin
extension which matches the source in compile.sh
but not the destination. Please maintain consistency across all files.
Also applies to: 17-17
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
configuration/boards/ldo-nitehawk-36/compile.sh (1)
1-5
: Improve error handling in root privilege check.While the root privilege check is correct, consider these improvements:
- Add a more descriptive error message explaining why root is required.
- Use a non-zero exit code to indicate failure.
#!/bin/bash if [ "$EUID" -ne 0 ] - then echo "ERROR: Please run as root" - exit + then echo "ERROR: Root privileges required for firmware compilation and file operations" + exit 1 fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
configuration/boards/ldo-nitehawk-36/compile.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/flash.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/make-and-flash-mcu.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/toolboard-config.cfg
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- configuration/boards/ldo-nitehawk-36/make-and-flash-mcu.sh
- configuration/boards/ldo-nitehawk-36/flash.sh
- configuration/boards/ldo-nitehawk-36/toolboard-config.cfg
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint, Typecheck and Tests
🔇 Additional comments (2)
configuration/boards/ldo-nitehawk-36/compile.sh (2)
12-16
: LGTM!The directory management code follows best practices by checking existence before creation and setting appropriate permissions.
17-19
: LGTM!The file extension issue from the previous review has been resolved, and the code correctly handles file copying and permissions.
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config | ||
pushd /home/pi/klipper || exit | ||
make olddefconfig | ||
make clean | ||
make |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for make commands.
The make commands should check for errors to prevent continuing with a failed build.
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config
pushd /home/pi/klipper || exit
-make olddefconfig
-make clean
-make
+make olddefconfig || exit 1
+make clean || exit 1
+make || exit 1
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config | |
pushd /home/pi/klipper || exit | |
make olddefconfig | |
make clean | |
make | |
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config | |
pushd /home/pi/klipper || exit | |
make olddefconfig || exit 1 | |
make clean || exit 1 | |
make || exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
configuration/boards/ldo-nitehawk-36/compile.sh (2)
1-5
: Improve error handling in root privilege check.The error message could be more descriptive, and an explicit exit code should be used.
Apply this diff to improve error handling:
#!/bin/bash if [ "$EUID" -ne 0 ] - then echo "ERROR: Please run as root" - exit + then echo "ERROR: This script requires root privileges. Please run with sudo." + exit 1 fi
12-16
: Reduce code duplication by using variables.Consider using variables for repeated paths to improve maintainability.
Apply this diff to reduce duplication:
+FIRMWARE_DIR="/home/pi/printer_data/config/firmware_binaries" + -if [ ! -d "/home/pi/printer_data/config/firmware_binaries" ] +if [ ! -d "$FIRMWARE_DIR" ] then - mkdir /home/pi/printer_data/config/firmware_binaries - chown pi:pi /home/pi/printer_data/config/firmware_binaries + mkdir "$FIRMWARE_DIR" + chown pi:pi "$FIRMWARE_DIR" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
configuration/boards/ldo-nitehawk-36/98-ldo-nitehawk-36.rules
(1 hunks)configuration/boards/ldo-nitehawk-36/compile.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/firmware.config
(1 hunks)configuration/boards/ldo-nitehawk-36/flash.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/make-and-flash-mcu.sh
(1 hunks)configuration/boards/ldo-nitehawk-36/toolboard-config.cfg
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- configuration/boards/ldo-nitehawk-36/flash.sh
- configuration/boards/ldo-nitehawk-36/toolboard-config.cfg
- configuration/boards/ldo-nitehawk-36/make-and-flash-mcu.sh
- configuration/boards/ldo-nitehawk-36/98-ldo-nitehawk-36.rules
- configuration/boards/ldo-nitehawk-36/firmware.config
🔇 Additional comments (2)
configuration/boards/ldo-nitehawk-36/compile.sh (2)
7-10
: LGTM! Error handling has been implemented correctly.The make commands now include proper error handling, and directory navigation is safely managed.
17-19
: LGTM! File extension and ownership are handled correctly.The firmware is copied with the correct
.bin
extension, and proper ownership is set.
then echo "ERROR: Please run as root" | ||
exit | ||
fi | ||
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for configuration file copy.
The file copy operation should verify the existence of source files and handle errors.
Apply this diff to improve error handling:
-cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config
+if [ ! -f "/home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config" ]; then
+ echo "ERROR: firmware.config not found"
+ exit 1
+fi
+cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config || {
+ echo "ERROR: Failed to copy firmware.config"
+ exit 1
+}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config | |
if [ ! -f "/home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config" ]; then | |
echo "ERROR: firmware.config not found" | |
exit 1 | |
fi | |
cp -f /home/pi/printer_data/config/RatOS/boards/ldo-nitehawk-36/firmware.config /home/pi/klipper/.config || { | |
echo "ERROR: Failed to copy firmware.config" | |
exit 1 | |
} |
Everything included.
As soon as I get the hosted mage I will update wiring diagrams
Summary by CodeRabbit