Skip to content

Altium empty comments#15

Merged
hydra merged 2 commits into
MakerPnP:masterfrom
andreyplus:altium-comments
Apr 17, 2026
Merged

Altium empty comments#15
hydra merged 2 commits into
MakerPnP:masterfrom
andreyplus:altium-comments

Conversation

@andreyplus

@andreyplus andreyplus commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

Added support for Altium empty comments (G04*)

G04*
G04 #@! TF.GenerationSoftware,Altium Limited,Altium Designer,25.8.1 (18)*
G04*

Many G-code variants generated by Altium are not currently supported; significant improvements are required in the future.

@nicolube

nicolube commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@andreyplus can u pls open issues what exactly is not supported?

@hydra

hydra commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@nicolube The missing space between the G04 and the *. With reference to 4.1 Comment (G04) in the 2024.05 spec.

@nicolube

nicolube commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@andreyplus This is for u...
@hydra This is not my PR

@hydra

hydra commented Apr 17, 2026

Copy link
Copy Markdown
Contributor
  1. I don't think we need a specific test for altium, since that adds quite a lot of code. I feel a simple comment in the test for the scenario might be better.

Please combine the G04* test into G04_comment_attributes and add a rust comment and note that altium doesn't follow the 4.1 Comment (G04) in the 2024.05 spec. (as I understand it).

  1. There is missing a test for the following scenario:
G04 *

Please update the existing comment test G04_comment_attributes so the test shows the expected comment is an empty string.

@hydra

hydra commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@andreyplus This is for u... @hydra This is not my PR

yes, I understood that, I was just replying to your question on behalf of @andreyplus 😄

@nicolube

Copy link
Copy Markdown
Contributor

yes, I understood that, I was just replying to your question on behalf of @andreyplus 😄

I thought he meant, besides G4 there is other stuff then is currently not supportet

@andreyplus

Copy link
Copy Markdown
Contributor Author

@nicolube @hydra Done

@hydra hydra merged commit 4c1a05b into MakerPnP:master Apr 17, 2026
4 checks passed
@hydra

hydra commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@hydra Done

Awesome, yeah, much more succinct and maintainable now. Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants