Rat-OS / RatOS

The preconfigured Raspberry Pi image that makes it easy to run Klipper + Moonraker + Mainsail on your printer.
https://os.ratrig.com
GNU General Public License v3.0
190 stars 119 forks source link

Problem with Includes #78

Closed Kaelum closed 1 year ago

Kaelum commented 1 year ago

What happened

There is an inherent problem when using a system that can only add or replace values, as you can't remove a key-value pair once introduced. This means that for every option that requires you to use a different key-value pair, you need to double the number of includes. This becomes unwieldly, is impossible to manage, and is impossible for users to use.

With your current configuration it is completely impossible to use CAN, as the baud and serial keys under [mcu] will cause Klipper to fail. This is just 1 of many situations that I ran into while attempting to use RatOS. In the end I had to stop using your templates entirely, and instead copy and paste the required sections into the printer.cfg file, which is not a good option.

What did you expect to happen

This should be easier to manage. The only possible way for this to work would require that Klipper provides an option for removing key-value pairs from the configuration after they've been defined.

How to reproduce

(see What happened)

Additional information

No response

joaobarros commented 1 year ago

I talked to Mikkel about this, he has talked to Kevin about it. This is a known "issue", but it's a Klipper issue so you should start a discussion over Klipper's Discourse about it.

miklschmidt commented 1 year ago

The only way, if you really insist on using CAN, is to define your own board config. Copy the existing one, replace the mcu section. For everything else, this is little more than an inconvenience really. It's just how it is for now.

aardvarkglory commented 1 year ago

This would be an easy fix. Just remove the [mcu] lines out of the config files in the printer folder and add it in the printer.cfg. This would allow everyone more flexibility in configuring what they would like to use. This is the one issue that forces me to Mainsail but would really like to use RatOS. Made a pull request about this a week ago here: https://github.com/Rat-OS/RatOS-configuration/pull/125

joaobarros commented 1 year ago

I'll start by saying I'm all up for allowing customizations but 90+% of users do not need the proposed changes You would double the number of lines of the already big list of boards present in printer.cfg

On some of my systems where I need some overrides over the standard files that I cannot unset, I clone the board file I need changed and include the clone in printer.cfg and comment the standard file.

miklschmidt commented 1 year ago

@aardvarkglory yeah no, i'm not gonna give the user more to worry about for no apparent reason. You can just copy and paste the board config if you're doing stuff that's out of scope, i don't see the big deal here. In all normal use cases you do not have to touch this.

I'm closing this for now.

miklschmidt commented 1 year ago

To follow up on this. There is no baud rate set in any of the board config files, so part of the initial problem in this issue does not exist. Just add the baud rate and serial to user override section like everything else.

## USER OVERRIDES
[mcu]
baud: 250000
serial: /whatever/you/want

If you need to unset the serial, copy/paste the board config. As usual, i do not recommend using CAN (zero advantages, worse user experience), and it is outside the scope of RatOS.

Kaelum commented 1 year ago

Here is the contents of RatOS/boards/btt-octopus-pro-446/config.cfg, which is exactly like every other board configuration file:

# WARNING. DO NOT EDIT THIS FILE.
# To override settings from this file, you can copy and paste the relevant
# sections into your printer.cfg and change it there.

[board_pins octopus_pro_446_tmc2209]
aliases:
# steppers
  x_step_pin=PF13,   x_dir_pin=PF12,   x_enable_pin=PF14,   x_uart_pin=PC4,   x_diag_pin=PG6,   x_endstop_pin=PG6,
  y_step_pin=PG0,   y_dir_pin=PG1,   y_enable_pin=PF15,   y_uart_pin=PD11,   y_diag_pin=PG9,   y_endstop_pin=PG9,
  z0_step_pin=PC13,  z0_dir_pin=PF0,  z0_enable_pin=PF1,  z0_uart_pin=PE4,  z0_diag_pin=null,
  z1_step_pin=PE2,  z1_dir_pin=PE3,  z1_enable_pin=PD4,  z1_uart_pin=PE1,  z1_diag_pin=null,
  z2_step_pin=PE6,  z2_dir_pin=PA14,  z2_enable_pin=PE0,  z2_uart_pin=PD3,  z2_diag_pin=null,
  z3_step_pin=PF9,  z3_dir_pin=PF10,    z3_enable_pin=PG2,  z3_uart_pin=PF2, z3_diag_pin=null, # Voron 2.4 support
  e_step_pin=PF11,   e_dir_pin=PG3,   e_enable_pin=PG5,   e_uart_pin=PC6,   e_diag_pin=null,   e_heater_pin=PA2,  e_sensor_pin=PF4,
  stepper_spi_mosi_pin=PA7,  stepper_spi_miso_pin=PA6,  stepper_spi_sclk_pin=PA5,
# accel
  adxl345_cs_pin=PA15,
# auto leveling
  bltouch_sensor_pin=PB7,  bltouch_control_pin=PB6,
  probe_pin=PB7,
# fans
  fan_part_cooling_pin=PA8,
  fan_toolhead_cooling_pin=PE5,
  fan_controller_board_pin=PD12,
# Bed heater
  heater_bed_heating_pin=PA1,
  heater_bed_sensor_pin=PF3,

## Expansion ports
  # EXP1 header
  EXP1_1=PE8, EXP1_3=PE9, EXP1_5=PE12, EXP1_7=PE14, EXP1_9=<GND>,
  EXP1_2=PE7, EXP1_4=PE10, EXP1_6=PE13, EXP1_8=PE15, EXP1_10=<5V>,
  # EXP2 header
  EXP2_1=PA6, EXP2_3=PB1, EXP2_5=PB2, EXP2_7=PC15,  EXP2_9=<GND>,
  EXP2_2=PA5, EXP2_4=PA4, EXP2_6=PA7, EXP2_8=<RST>, EXP2_10=PC5,
  # Pins EXP2_1, EXP2_6, EXP2_2 are also MISO, MOSI, SCK of bus "spi2"

[mcu]
baud: 250000
serial: /dev/btt-octopus-pro-446

[temperature_sensor Octopus_Pro_446]
sensor_type: temperature_mcu
min_temp: 0
max_temp: 100

[adxl345]
spi_bus: spi3
cs_pin: adxl345_cs_pin
miklschmidt commented 1 year ago

Baud rate was removed from all files yesterday, it was in way more files than i remember, i think it made it in through copy pasting. It has no purpose since it's just the default 25000 anyway, so it's gone.

What else seems to be the problem?

aardvarkglory commented 1 year ago

Here is a solution that does not change anything for the average user but at the same time gives the advanced user what he wants. Add config file to each of the board folders and call it config-nomcu.cfg. It would have everything that the normal confic.cfg file has minus the [mcu] lines. All an advanced user would have to do is add (-nomcu) to the printer of choice in the printer.cfg file.

Another way to do this is to make the board mcu a variable (board_mcu=/dev/btt-octopus-pro-429). Then in the printer.cfg file you call it in the [mcu] section. If someone wants to change that, they will have that power and nothing is broken. Just put a note above [mcu] stating any change to this value should not be made.