CuraEngine icon indicating copy to clipboard operation
CuraEngine copied to clipboard

Plugin gcode modify patches

Open ThomasRahm opened this issue 2 years ago • 4 comments

Description

CuraEngine currently does not set per model settings to a the GcodePathModify slot. Also if fan speed is changed for travel paths, the changed fan speed will not be applied. This fixes both of these issues. Also it adds the a "mesh_name" setting to the per-model settings, so that the mesh can later be identified using its name. To my knowledge the mesh name is unique as if the same model is added multiple times a counter is appended by the frontend (e.g. Example.stl and Example.stl(1) and so forth).

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)

Checklist:

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

ThomasRahm avatar Nov 09 '23 09:11 ThomasRahm

Resolved the merge conflicts from the github website ui, soooo if i have broken anything sowry

casperlamboo avatar Jan 31 '24 12:01 casperlamboo

Hi @ThomasRahm ,

Thanks for you contribution! Heard about this oversight indeed from @rburema, good initiative to fix it!

I have a question though

CuraEngine currently does not set per model settings to a the GcodePathModify slot. ... Also it adds the a "mesh_name" setting to the per-model settings, so that the mesh can later be identified using its name.

This last remark seems to me a bit like a workaround of the issue above. If we were to send over the the model_id with the modify call, then we would be able to retrieve the correct settings from the correct model setting stack?

casperlamboo avatar Jan 31 '24 12:01 casperlamboo

It definitely is a bit of a workaround.

Every path seems to be able to belong to a different model, so it would need to be sent as part of each path. Also I do not know if the RepeatedPtrField returned by BroadcastServiceSettingsRequest::object_settings() is ordered (As in the settings of model 3 are at position 3). A model id is in my opinion better to identify models than the name, I just do not know how I would be extending the API to include it.

Also want to note that this pull request merges (for plugins) mesh groups settings with the per object settings on engine side before they are being sent to the plugin. If this is not wanted one would even need to also transmit an identifier for the mesh group the model is part of and transmit the mesh group settings separately.

ThomasRahm avatar Jan 31 '24 14:01 ThomasRahm

Hi Thomas,

We're in the progress of updating our Way-Of-Working with regards to PR's so I'm adding some label(s). That way we can prioritize our work-load and give your PR the love it deserves.

Either I our one of my colleagues will pick this up in the near future, but I think @casperlamboo is already doing this

jellespijker avatar Feb 16 '24 09:02 jellespijker