Skip to content
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

3 new Multitool configuration options #2114

Merged
merged 3 commits into from
Oct 10, 2024
Merged

3 new Multitool configuration options #2114

merged 3 commits into from
Oct 10, 2024

Conversation

TheSin-
Copy link
Contributor

@TheSin- TheSin- commented Jul 6, 2024

Description

  • Add bool to set if the start_gcode needs to be the first gcode run. Requires machine_start_gcode_first be added to the machine UI as a checkbox. Having T0 called before the start gcode on a changer isn't wise as the start code needs to initialize the system and level before a tool change happens. This is an option to correct that behaviour for those that require it.
  • Add a prestart_gcode to extruders, this will allow use set temperatures before the change call so we can preheat the next extruder before the change call to save change times. Requires machine_extruder_prestart_code text box be added to the extruder UI page.
  • Add a time duration to calculate time required to change extruders. Requires machine_extruder_change_duration float box to be added to the extruder UI page.
  • Fix a bug in the export code that was checking num_extruders instead of used_extruders, this was adding a tool change on start to T0 when only one extruder is used which we do not want, with one extruder it should output like normal and let the user choose the extruder they want to use.

Required for Ultimaker/Cura#19333

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

NOTE not tested yet as I'm still learning how to build a custom engine into a full build

Not tested yet, I need to slice and check the resulting gcode, it should move start gcode before the T0 call, Should add the restart gcode before all toolchanges (This one might need an exemption on the first T0).

Test Configuration:

  • Operating System: MacOS

Checklist:

  • My code follows the style guidelines of this project as described in UltiMaker Meta
  • I have read the Contribution guide
  • I have commented my code, particularly in hard-to-understand areas
  • I have uploaded any files required to test this change

@github-actions github-actions bot added the PR: Community Contribution 👑 Community Contribution PR's label Jul 6, 2024
@TheSin- TheSin- mentioned this pull request Jul 6, 2024
8 tasks
@nallath
Copy link
Member

nallath commented Aug 5, 2024

Looks good to me. Have you been able to test if this works already?

@TheSin-
Copy link
Contributor Author

TheSin- commented Aug 7, 2024

I will be testing it this weekend, thanks for looking at it.

@TheSin-
Copy link
Contributor Author

TheSin- commented Aug 7, 2024

finally figured out how to build it (not the proper way but enough for testing) and my code does not work at all

I'm still getting gcode before the START

;Generated with Cura_SteamEngine 5.8.0-alpha.0+source
T0
M104 S260
M105
M109 S260
PRINT_PRESTART BED_TEMP=110.0 CHAMBER_TEMP=70
PRINT_START WZ=0.2 WN=0.4 BED_TEMP=110.0 EXTRUDER_TEMP=260.0
M82 ;absolute extrusion mode

The M82 was moved down but the rest I need to figure out.

My Pre Change code isn't expanding

G92 E0
M104 S{material_print_temperature} T{extruder_nr}
T0

So I have stuff to figure out but at least now I can work on it, I'll push fixes as soon as I have it working.

- Add bool to set if the start_gcode needs to be the first gcode run.  Requires `machine_start_gcode_first` be added to the machine UI as a checkbox
- Add a prestart_gcode to extruders, this will allow use set temperatures before the change call so we can preheat the next extruder.  Requires `machine_extruder_prestart_code` text box be added to the extruder UI page.
- Add a time duration to calculate time required to change extruders. Requires `machine_extruder_change_duration` float box to be added to the exturder UI page.
- Fix a bug in the export code that was checking num_extruders instead of used_extruders, this was adding a tool change on start to T0 when only one extruder is used which we do not want, with one extruder it should output like normal and let the user choose the extruder they want to use.
@TheSin-
Copy link
Contributor Author

TheSin- commented Aug 10, 2024

Tested and working

@TheSin-
Copy link
Contributor Author

TheSin- commented Aug 10, 2024

Updated for the latest code changes and retested

@nallath nallath merged commit 82a4986 into Ultimaker:main Oct 10, 2024
17 of 18 checks passed
@nallath
Copy link
Member

nallath commented Oct 10, 2024

Sorry for that! I missed the notification that you fixed & tested the engine part.

@TheSin-
Copy link
Contributor Author

TheSin- commented Nov 19, 2024

No problem, I noticed it wasn't in 5.9 did it just miss the Freeze?

@rburema
Copy link
Member

rburema commented Nov 26, 2024

(Also) For future reference -- please check if the unit-tests actually work. We've got that ✔️ / ❌ for a reason 😉

In this case, it's because for the tests, the settings used there will need sensible default values -- in the relevant test these are defined in TEST_F(GCodeExportTest, SwitchExtruderSimple).

I'll make a commit in main after I'll finish this comment.

@rburema
Copy link
Member

rburema commented Nov 26, 2024

@TheSin- Yeah just about, we entered string & feature freeze on the 8th of October. -- Somewhat related: Please also note that for most of this year the Cura-team is working with a reduced team-size (which will hopefully clear up sometime early next year...)

@TheSin-
Copy link
Contributor Author

TheSin- commented Nov 26, 2024

(Also) For future reference -- please check if the unit-tests actually work. We've got that ✔️ / ❌ for a reason 😉

In this case, it's because for the tests, the settings used there will need sensible default values -- in the relevant test these are defined in TEST_F(GCodeExportTest, SwitchExtruderSimple).

I'll make a commit in main after I'll finish this comment.

do you have a link to the preferred method of running the tests, I'm more then happy to do so but I figured the CI/CD would do that for me

@rburema
Copy link
Member

rburema commented Nov 27, 2024

@TheSin- The CI/CD does do that -- you can click on the ❌ to see what went wrong -- I'm just saying that we should be more careful what we merge.

If you want to run any of them locally, they should already have been built in ./build/<release-mode>/tests unless you build with skip_tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Community Contribution 👑 Community Contribution PR's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants