Feature Request: generate-profile builds diff

[evilC]

As a user
I want to be able to take an existingly modified version of Marlin, plus the original unmodified Marlin source that it was derived from, and generate a profile yaml file which only contains the changes that were made to the unmodified source to get to the modified version.
So that I can take my existing modified marlin and create as minimal a profile.yaml as possible

TLDR, I think that while the generate-profile command is nice, it creates a lot of noise - you end up with every single constant and the current value (Be it the original value or your new one), when all you really are interested in is what you changed

Given an unaltered Marlin folder Marlin
And a modified Marlin folder Modified
When I execute a command similar to marlin-console-configurator generate-profile ./Marlin ./Modified -d ./diff.yaml
Then diff.yaml is created with ONLY the differences between Marlin and Modified

ie:
Original Marlin source (Marlin\Configuration.h):

//#define CUSTOM_MACHINE_NAME "3D Printer"
#define SOME_CONSTANT "Foo"

Modified Marlin source (Modified\Configuration.h)

#define CUSTOM_MACHINE_NAME "MyPrinterName"
#define SOME_CONSTANT "Foo"

Output (diff.yaml):

enabled:
   CUSTOM_MACHINE_NAME: '"MyPrinterName"'

[Chuckame]

Hello,

Thank you for this interesting feedback.

This idea seems good, and you've right, generate profile outputs too much config...

Fyi: You have a command, that is not generating a profile, but can display differences between 2 folders / files : diff

I will take for how to make your request!

[jbellmore]

I totally agree with this feature!

What I did to try to accomplish the same is to run the generate profile command on a fresh marlin, then run it again on the modified marlin and do a diff. That seemed to work reasonably well but I am not positive if this will work properly and here is why.

When I generate a profile from standard unmodified Marlin and then do an apply using that profile I would expect to see zero changes, however there are 49 changes in the adv config file and 5 in the standard config file.

Here are the changes in the adv file:

Change          CHAMBER_FAN_BASE: 255 → 128
Change          PID_FAN_SCALING_MIN_SPEED: 10.0 → 10
Enable          DGUS_UI_MOVE_DIS_OPTION
Change          BLOCK_BUFFER_SIZE: 8 → 16
Change          X_MICROSTEPS: 16 → 128
Change          X2_MICROSTEPS: 16 → 128
Change          Y_MICROSTEPS: 16 → 128
Change          Y2_MICROSTEPS: 16 → 128
Change          Z_MICROSTEPS: 16 → 128
Change          Z2_MICROSTEPS: 16 → 128
Change          Z3_MICROSTEPS: 16 → 128
Change          Z4_MICROSTEPS: 16 → 128
Change          E0_MICROSTEPS: 16 → 128
Change          E1_MICROSTEPS: 16 → 128
Change          E2_MICROSTEPS: 16 → 128
Change          E3_MICROSTEPS: 16 → 128
Change          E4_MICROSTEPS: 16 → 128
Change          E5_MICROSTEPS: 16 → 128
Change          E6_MICROSTEPS: 16 → 128
Change          E7_MICROSTEPS: 16 → 128
Change          X_MICROSTEPS: 16 → 128
Change          X2_MICROSTEPS: 16 → 128
Change          Y_MICROSTEPS: 16 → 128
Change          Y2_MICROSTEPS: 16 → 128
Change          Z_MICROSTEPS: 16 → 128
Change          Z2_MICROSTEPS: 16 → 128
Change          Z3_MICROSTEPS: 16 → 128
Change          Z4_MICROSTEPS: 16 → 128
Change          E0_MICROSTEPS: 16 → 128
Change          E1_MICROSTEPS: 16 → 128
Change          E2_MICROSTEPS: 16 → 128
Change          E3_MICROSTEPS: 16 → 128
Change          E4_MICROSTEPS: 16 → 128
Change          E5_MICROSTEPS: 16 → 128
Change          E6_MICROSTEPS: 16 → 128
Change          E7_MICROSTEPS: 16 → 128
Change          SPINDLE_LASER_POWERUP_DELAY: 5000 → 50
Change          SPINDLE_LASER_POWERDOWN_DELAY: 5000 → 50
Change          SPEED_POWER_MIN: 5000 → 0
Change          SPEED_POWER_MAX: 30000 → 100
Change          SPEED_POWER_STARTUP: 25000 → 80

A lot of them are not straightforward setting this value because they are conditional based on other things being set so I think the logic is confused about those types. When I actually do the apply it changes the values within the conditionals which it really shouldn't be doing. I think it is evaluating what the value is ultimately set to but when applying it just finds the first instance and updates that value. Here is an example code block that is being updated:

//#define CHAMBER_FAN               // Enable a fan on the chamber
  #if ENABLED(CHAMBER_FAN)
    #define CHAMBER_FAN_MODE 2        // Fan control mode: 0=Static; 1=Linear increase when temp is higher than target; 2=V-shaped curve.
    #if CHAMBER_FAN_MODE == 0
      #define CHAMBER_FAN_BASE  128   // Chamber fan PWM (0-255)
    #elif CHAMBER_FAN_MODE == 1
      #define CHAMBER_FAN_BASE  128   // Base chamber fan PWM (0-255); turns on when chamber temperature is above the target
      #define CHAMBER_FAN_FACTOR 25   // PWM increase per °C above target
    #elif CHAMBER_FAN_MODE == 2
      #define CHAMBER_FAN_BASE  128   // Minimum chamber fan PWM (0-255)
      #define CHAMBER_FAN_FACTOR 25   // PWM increase per °C difference from target
    #endif
  #endif

When chamber mode is 0 the fan is supposed to be set to 255 and it changed that next line to 128. So I think the diff will get you some of the way there but you will have to remove some of the values like the example shown which are really false positives of changes.

Obviously this logic won't be super easy to automate but hopefully it gets us a lot of the way.

[jbellmore]

Just an example of the output of doing this I took the stock Marlin and the default Ender 3 profile, generated two profiles and performed the diff and here is what I get:

enabled:
  STRING_CONFIG_H_AUTHOR: "\"(thisiskeithb, Ender-3)\""
  SHOW_CUSTOM_BOOTSCREEN: null
  CUSTOM_STATUS_SCREEN_IMAGE: null
  BAUDRATE: 115200
  MOTHERBOARD: BOARD_MELZI_CREALITY
  CUSTOM_MACHINE_NAME: '"Ender-3"'
  BED_MAXTEMP: 125
  PID_EDIT_MENU: null
  PID_AUTOTUNE_MENU: null
  DEFAULT_Kp_LIST: "{  21.73,  21.73 }"
  DEFAULT_Ki_LIST: "{   1.54,   1.54 }"
  DEFAULT_Kd_LIST: "{  76.55,  76.55 }"
  DEFAULT_Kp: 21.73
  DEFAULT_Ki: 1.54
  DEFAULT_Kd: 76.55
  DEFAULT_AXIS_STEPS_PER_UNIT: "{ 80, 80, 400, 93 }"
  DEFAULT_MAX_FEEDRATE: "{ 500, 500, 5, 25 }"
  DEFAULT_MAX_ACCELERATION: "{ 500, 500, 100, 5000 }"
  DEFAULT_ACCELERATION: 500
  DEFAULT_RETRACT_ACCELERATION: 500
  DEFAULT_TRAVEL_ACCELERATION: 500
  JUNCTION_DEVIATION_MM: 0.08
  INVERT_X_DIR: "true"
  INVERT_E0_DIR: "true"
  X_BED_SIZE: 235
  Y_BED_SIZE: 235
  Z_MAX_POS: 250
  HOMING_FEEDRATE_XY: (20*60)
  EEPROM_SETTINGS: null
  PREHEAT_1_TEMP_HOTEND: 185
  PREHEAT_1_TEMP_BED: 45
  PREHEAT_1_FAN_SPEED: 255
  PREHEAT_2_TEMP_BED: 70
  PREHEAT_2_FAN_SPEED: 255
  SDSUPPORT: null
  SPEAKER: null
  CR10_STOCKDISPLAY: null
  QUICK_HOME: null
  BLTOUCH_SET_5V_MODE: null
  Z_STEPPER_ALIGN_ITERATIONS: 3
  LCD_INFO_MENU: null
  STATUS_MESSAGE_SCROLLING: null
  SDCARD_READONLY: null
  SCROLL_LONG_FILENAMES: null
  BABYSTEPPING: null
  DOUBLECLICK_FOR_Z_BABYSTEPPING: null
  X_CURRENT: 580
  Y_CURRENT: 580
  Z_CURRENT: 580
  E0_CURRENT: 650
  CHOPPER_TIMING: CHOPPER_DEFAULT_24V

disabled:
  - DISABLE_INACTIVE_EXTRUDER
  - TFT_320x240
  - TFT_320x240_SPI
  - TFT_480x320
  - TFT_480x320_SPI
  - TFT_DRIVER
  - SPI_GRAPHICAL_TFT
  - FSMC_GRAPHICAL_TFT
  - TFT_LVGL_UI_FSMC
  - TFT_LVGL_UI_SPI
  - GRAPHICAL_TFT_ROTATE_180

Basically I just grab whatever is added to the enabled section and whatever is added to the disabled. If something is added from enabled it should also be shown as removed in the diff but really you should only need those listed as adds to avoid running into the situation listed above where something hasn't been really changed from default.

When I run the apply with just those diffs to the ender3 configuration of marlin I get zero updates shown to apply, which seems like it is working properly with those diffs.

Even if it didn't have the other functionality using this tool to be able to get the diffs between firmware configs is awesome!

Thanks @Chuckame!

[Chuckame]

Hello there, great ideas, but please send what you've done with which files (executed command + upload your files as a code block).

@jbellmore not agree all of your idea 😄 this is not the goal of this tool to understand if a constant is in an actived if block. The goal is to keep it stupid and simple (KISS), so you can do it in an other way: make a profile for general config, and an other for the chamber config. And when you want to activate the chamber, just apply the common config + the chamber config. And if you don't want it, just don't apply it 👌

For the generation of a "diff" profile, good idea, I think I will add a generate-diff-profile taking --from and --to params, allowing you to generate a profile only corresponding to changes that you've made.

@jbellmore please create another issue corresponding to your problem/expectation, this will be more simple for me to follow 😁

[Chuckame]

Or I just change the current diff command to replace current outputs by a diff profile in yaml?
What do you think? And have the same as apply command, that display changes, and if --save is present, the diff profile is generated where we want!

[jbellmore]

Hey sorry if I was unclear above. After re-reading this thread I definitely didn't mean to imply it should solve that problem just that doing the diff will require the original unmodified marlin source to be able to compare since without that you won't be able to tell which values are indeed changed or not without some massive effort to interpret the source code, which obviously I don't think is something this project should undertake!

My point was to try to illustrate that you can't simply take the generated profile and re-apply that or else you will likely get unintended source modifications.

After re-reading the original post it appears I am saying the say thing as @evilC 😄

So I would agree the diff just needs to be the diff of the 2 generated config files, the one generated by unmodified marlin and the one generated by the modified source.

So I would think it would depend on the intent of what you want the diff and generate-profile commands to be used for. If you intend diff to just be used to spit diffs to screen then maybe modify the generate-profile command to be able to take in --left and --right as the diff command does.

The other option would be to modify the diff command to be able to take an optional output file to save the diff to in yml format.

I think both would be completely reasonable and have no strong opinions either way!

Thanks again!

[Chuckame]

@evilC @jbellmore can you test the new pull request ? Tell me if ok for you 😄

[jbellmore]

Just tested this one also, seems to work great!

I ran this against Vanilla 2.0 Marlin and the stock Bigtreetech SKR Mini E3 V2 firmware and got the following output:

enabled:
  Y_SLAVE_ADDRESS: 0
  Z_MAX_POS: 200
  FILAMENT_CHANGE_UNLOAD_LENGTH: 100
  E0_SLAVE_ADDRESS: 0
  DEFAULT_Kd_LIST: "{ 114.00, 114.00 }"
  EXTRUDE_MAXLENGTH: 200
  Y_CURRENT: 800
  PREHEAT_1_TEMP_BED: 70
  DEFAULT_AXIS_STEPS_PER_UNIT: "{ 80, 80, 4000, 500 }"
  E0_CURRENT: 800
  NEOPIXEL_TYPE: NEO_GRBW
  CHOPPER_TIMING: CHOPPER_DEFAULT_12V
  MOTHERBOARD: BOARD_RAMPS_14_EFB
  Z_SLAVE_ADDRESS: 0
  DEFAULT_Ki_LIST: "{   1.08,   1.08 }"
  INVERT_X_DIR: "false"
  NOZZLE_TO_PROBE_OFFSET: "{ 10, 10, 0 }"
  Y_STALL_SENSITIVITY: 8
  PREHEAT_1_FAN_SPEED: 0
  DEFAULT_RETRACT_ACCELERATION: 3000
  SDSORT_CACHE_NAMES: "false"
  BAUDRATE: 250000
  STRING_CONFIG_H_AUTHOR: "\"(none, default config)\""
  DEFAULT_Ki: 1.08
  DEFAULT_Kd: 114.00
  DEFAULT_Kp: 22.20
  JUNCTION_DEVIATION_MM: 0.013
  DEFAULT_TRAVEL_ACCELERATION: 3000
  X_STALL_SENSITIVITY: 8
  X_CURRENT: 800
  INVERT_E0_DIR: "false"
  ARC_SUPPORT:
  X_BED_SIZE: 200
  HOMING_FEEDRATE_XY: (50*60)
  Y_BED_SIZE: 200
  LIN_ADVANCE_K: 0.22
  PLR_ENABLED_DEFAULT: "false"
  DEFAULT_ACCELERATION: 3000
  Z_CURRENT: 800
  SDSORT_USES_RAM: "false"
  PREHEAT_2_FAN_SPEED: 0
  DEFAULT_Kp_LIST: "{  22.20,  22.20 }"
  DEFAULT_bedKp: 10.00
  DEFAULT_bedKi: .023
  DEFAULT_bedKd: 305.4
  DEFAULT_MAX_ACCELERATION: "{ 3000, 3000, 100, 10000 }"
  DEFAULT_MAX_FEEDRATE: "{ 300, 300, 5, 25 }"
  FILAMENT_CHANGE_FAST_LOAD_LENGTH: 0
  SERIAL_PORT: 0
  BED_MAXTEMP: 150
  PREHEAT_1_TEMP_HOTEND: 180
  NEOPIXEL_PIN: 4
disabled:
  - HOME_BEFORE_FILAMENT_CHANGE
  - LCD_FEEDBACK_FREQUENCY_HZ
  - SCROLL_LONG_FILENAMES
  - LCD_FEEDBACK_FREQUENCY_DURATION_MS
  - SDSUPPORT
  - FILAMENT_LOAD_UNLOAD_GCODES
  - EXTRAPOLATE_BEYOND_GRID
  - QUICK_HOME
  - X_DRIVER_TYPE
  - SDCARD_SORT_ALPHA
  - EMERGENCY_PARSER
  - IMPROVE_HOMING_RELIABILITY
  - POWER_LOSS_PURGE_LEN
  - Z_DRIVER_TYPE
  - LCD_INFO_MENU
  - REPORT_FAN_CHANGE
  - INDIVIDUAL_AXIS_HOMING_MENU
  - LONG_FILENAME_HOST_SUPPORT
  - ADVANCED_PAUSE_FEATURE
  - NOZZLE_PARK_FEATURE
  - EEPROM_SETTINGS
  - SQUARE_WAVE_STEPPING
  - USE_CONTROLLER_FAN
  - SPEAKER
  - S_CURVE_ACCELERATION
  - RESTORE_LEVELING_AFTER_G28
  - E0_DRIVER_TYPE
  - CR10_STOCKDISPLAY
  - PARK_HEAD_ON_PAUSE
  - LCD_SET_PROGRESS_MANUALLY
  - SERIAL_PORT_2
  - SDCARD_CONNECTION
  - CUSTOM_MACHINE_NAME
  - LIN_ADVANCE
  - Y_DRIVER_TYPE
  - MESH_EDIT_MENU
  - BABYSTEPPING
  - STATUS_MESSAGE_SCROLLING
  - POWER_LOSS_RECOVERY
  - DOUBLECLICK_FOR_Z_BABYSTEPPING
  - EEPROM_AUTO_INIT
  - CONTROLLER_FAN_EDITABLE
  - EXPERIMENTAL_SCURVE
  - SPI_GRAPHICAL_TFT
  - TFT_320x240
  - TFT_320x240_SPI
  - TFT_480x320
  - TFT_LVGL_UI_SPI
  - TFT_480x320_SPI
  - FSMC_GRAPHICAL_TFT
  - TFT_DRIVER
  - GRAPHICAL_TFT_ROTATE_180
  - TFT_LVGL_UI_FSMC

It is certainly way less than I would expect if it wasn't working at all. I don't know enough about the stock firmware values for the SKR (just getting started with it) but everything I am seeing looks reasonable to me! I am sure some of them are just differences in cases or spacing but I would certainly expect those to show up in the diff so seems to be working to me.

Thanks so much, this is incredibly useful!

[jbellmore]

Actually just an update, I apparently was doing the diff backwards, I had the vanilla Marlin as the --diff-from param. When I swapped the two source directories properly I get a diff that seems to make more sense.

I also tested by trying to apply to diff back to the SKR source and I get zero changes (as I would expect). When I apply the diff to vanilla Marlin I get a lot of changes.

So based that, it seems like it should be working correctly.

Thanks again!

[Chuckame]

Cool! I will merge the feature asap, and maybe use a real C language Parser to get grain fined values (like mathematical ops, or arrays). Thanks for your great feedback!