[Gerber Formatter]: Packed coordinate '42000000' is too long for the format(2,5).

[chenzanyu]

PyGerber version: 3.0.0a3
my gerber file content:

G04 ================== begin FILE IDENTIFICATION RECORD ==================*
G04 Layout Name:  mb-dcdc-m22kw.brd*
G04 Film Name:    Outline*
G04 File Format:  Gerber RS274X*
G04 File Origin:  Cadence Allegro 23.1-P001*
G04 Origin Date:  Tue Jan 14 13:39:39 2025*
G04 *
G04 Layer:  BOARD GEOMETRY/DESIGN_OUTLINE*
G04 *
G04 Offset:    (0.0000 0.0000)*
G04 Mirror:    No*
G04 Mode:      Positive*
G04 Rotation:  0*
G04 FullContactRelief:  No*
G04 UndefLineWidth:     0.1000*
G04 ================== end FILE IDENTIFICATION RECORD ====================*
%FSLAX25Y25*MOMM*%
%IR0*IPPOS*OFA0.00000B0.00000*MIA0B0*SFA1.00000B1.00000*%
%ADD10C,.1*%
G75*
%LPD*%
G75*
G54D10*
G01X42000000Y31000000D02*
Y0D01*
X0D01*
Y31000000D01*
X42000000D01*
M02*

[chenzanyu]

Please note that prior to this, there was another error: 'Expected end of text, found '%' (at char 563), (line:17, col:1)'.

[Argmaster]

This file contains multiple violations of Gerber format. In particular merging extended commands like this:

%FSLAX25Y25*MOMM*%

Is not acceptable and I don't think we can cover all of strange constructions one can include in Gerber files.

Now as for the error from the title it means that coordinates are incorrectly packed/padded, they are too long compared to what was declared in FS command. This is not valid Gerber code. I will consider adding some mechanism to ignore coordinate coding errors with truncation or sth, but right now there is nothing I can suggest except communicating to provider of your software that it producea incorrect code.

So, I will add workaround as todo, but ETA is currently impossible to determine due to amount of pending work for release 3.0.0. I will appreciate help, I can provide guidelines how to implement this workaround in PyGerber if you are interested in contributing.

[chenzanyu]

I believe you are correct, and this is an issue with the software vendor. However, please note that the file can be rendered normally when opened with KiCad or Gerbv.

[Argmaster]

I believe you are correct, and this is an issue with the software vendor. However, please note that the file can be rendered normally when opened with KiCad or Gerbv.

Ok, it is viable to add tolerance for this particular deviation from standard, at least at parsing level. However I do not have processing power to work on it right now. In meantime I will appreciate contributions with fixes and extensions to PyGerber if you would like to try working on it yourself.

[chenzanyu]

Please let me know what I should do. Once I have time, I will give it a try.

[Argmaster]

@chenzanyu sure, so there are at least two things necessary to be done to make this file work:

1

Parser must be informed about existence of constructions like %FSLAX25Y25*MOMM*% which are composed of multiple extended commands merged into one expression. Currently grammar treats % as part of a command, so %FSLAX25Y25*% is interpreted as one thing. If you have look into grammar definition for FS extended command here you will see that it is defined as monolith. Now in my opinion best way to tackle this edge case where single % pair wraps multiple extended commands is to extract each and every extended command inner grammar definition into separate function and create grammar expression using all of them.

Looking at code I linked above:

    def fs(self) -> pp.ParserElement:
        """Create a parser for the FS command."""
        return (
            self._extended_command(
                pp.Literal("FS")
                + pp.one_of(("L", "T", "")).set_results_name("zeros")
                + pp.one_of(("I", "A")).set_results_name("coordinate_mode")
                + pp.CaselessLiteral("X")
                + pp.Regex(r"[0-9]").set_results_name("x_integral")
                + pp.Regex(r"[0-9]").set_results_name("x_decimal")
                + pp.CaselessLiteral("Y")
                + pp.Regex(r"[0-9]").set_results_name("y_integral")
                + pp.Regex(r"[0-9]").set_results_name("y_decimal")
            )
            .set_parse_action(self.make_unpack_callback(FS))
            .set_name("FS")
        )

Following change would be required first:

    def fs(self) -> pp.ParserElement:
        """Create a parser for the FS command."""
        return (
            self._extended_command(self.fs_inner())
            .set_parse_action(self.make_unpack_callback(FS))
            .set_name("FS")
        )

    def fs_inner(self) -> pp.ParserElement:
        """Create a parser for the inner part of FS command."""
        return (
            pp.Literal("FS")
            + pp.one_of(("L", "T", "")).set_results_name("zeros")
            + pp.one_of(("I", "A")).set_results_name("coordinate_mode")
            + pp.CaselessLiteral("X")
            + pp.Regex(r"[0-9]").set_results_name("x_integral")
            + pp.Regex(r"[0-9]").set_results_name("x_decimal")
            + pp.CaselessLiteral("Y")
            + pp.Regex(r"[0-9]").set_results_name("y_integral")
            + pp.Regex(r"[0-9]").set_results_name("y_decimal")
        )

    def fs_no_bounds(self) -> pp.ParserElement:
        """Create a parser for the FS command."""
        return (
            self._command(self.fs_inner())
            .set_parse_action(self.make_unpack_callback(FS))
            .set_name("FS")
        )

To find all extended commands you can either grep _extended_command or check it in standard. I don't think we should include any other extended commands than just ones which change state, like FS, MO, IR etc.

Then you can create merged extended command parser:

    def merged_extended_command(self) -> pp.ParserElement:
        """Create a parser for two or more merged extended commands."""
        return (
            self._extended
            + pp.OneOrMore(
                pp.MatchFirst(
                    [
                        self.fs_no_bounds(),
                        # you should include all of extended commands here.
                    ]
                )
            )
            + self._extended
        )

Please place merged_extended_command at the very end of Grammar class definition.

To make parser aware of the grammar you have just created you will have to add it in build() method call:

    def build(self) -> pp.ParserElement:
        """Build the grammar."""

        def _(s: str, loc: int, tokens: pp.ParseResults) -> File:
            return self.get_cls(File)(
                source_info=SourceInfo(source=s, location=loc, length=len(s) - loc),
                nodes=tokens.as_list(),
            )

        root = (
            pp.OneOrMore(
                pp.MatchFirst(
                    [
                        self.d_codes_standalone,
                        self.g_codes,
                        self.load_commands,
                        self.aperture(),
                        self.attribute,
                        self.properties,
                        self.m_codes,
                        self.merged_extended_command(),  # <-----------
                    ]
                )
            )
            .set_results_name("root_node")
            .set_parse_action(_)
        )

        if self.enable_packrat:
            root.enable_packrat(cache_size_limit=self.packrat_cache_size)

        if self.enable_debug:
            root.set_debug()

        return root

You will also have to add test case checking different combinations of merged commands to ensure no regressions in the future.

2

CoordinateFormat class needs a workaround, preferably configured via parser option, allowing for unpacking incorrectly packed coordinates.

I would recommend adding two fields to CoordinateFormat which can hold and callable invoked when coordinates are incorrectly packed, depending on whether they are too long or too short:

class CoordinateFormat(_StateModel):
    """Coordinate format information."""

    zeros: Zeros = Field(default=Zeros.SKIP_LEADING)
    coordinate_mode: CoordinateNotation = Field(default=CoordinateNotation.ABSOLUTE)

    x_integral: int = Field(default=2)
    x_decimal: int = Field(default=6)

    y_integral: int = Field(default=2)
    y_decimal: int = Field(default=6)

    on_too_short_coordinate: CoordinatePolicy = Field(
        default=throw_too_short_exception_policy
    )
    on_too_long_coordinate: CoordinatePolicy = Field(
        default=throw_too_long_exception_policy
    )

Here is Protocol class for type hinting and two basic implementations which throw exceptions:

class CoordinatePolicy(Protocol):
    """Handle too short coordinate string."""

    def __call__(
        self,
        coordinate_format: CoordinateFormat,
        coordinate: PackedCoordinateStr,
        integer: int,
        decimal: int,
    ) -> PackedCoordinateStr:
        """Handle too short coordinate string."""
        ...


def throw_too_long_exception_policy(
    coordinate_format: CoordinateFormat,  # noqa: ARG001
    coordinate: PackedCoordinateStr,
    integer: int,
    decimal: int,
) -> PackedCoordinateStr:
    """Handle too long coordinate string."""
    raise PackedCoordinateTooLongError(coordinate, integer, decimal)


def throw_too_short_exception_policy(
    coordinate_format: CoordinateFormat,  # noqa: ARG001
    coordinate: PackedCoordinateStr,
    integer: int,
    decimal: int,
) -> PackedCoordinateStr:
    """Handle too short coordinate string."""
    raise PackedCoordinateTooShortError(coordinate, integer, decimal)

Now in all places where PackedCoordinateTooLongError and PackedCoordinateTooShortError are raised, those handlers should be invoked instead, for excample in _unpack_preprocess(), changing this:

    def _unpack_preprocess(
        self, coordinate: PackedCoordinateStr, integer: int, decimal: int
    ) -> tuple[Literal["-", "+"], str]:
        if len(coordinate) <= 0:
            raise PackedCoordinateTooShortError(coordinate, integer, decimal)

        sign: Literal["+", "-"] = "+"

        if coordinate[0] in ("+", "-"):
            sign = coordinate[0]  # type: ignore[assignment]
            coordinate_str = coordinate[1:]
        else:
            coordinate_str = str(coordinate)

        if len(coordinate_str) > (integer + decimal):
            raise PackedCoordinateTooLongError(coordinate, integer, decimal)

        if len(coordinate_str) <= 0:
            raise PackedCoordinateTooShortError(coordinate, integer, decimal)

        return sign, coordinate_str

Into this:

    def _unpack_preprocess(
        self, coordinate: PackedCoordinateStr, integer: int, decimal: int
    ) -> tuple[Literal["-", "+"], str]:
        if len(coordinate) <= 0:
            self.on_too_short_coordinate(self, coordinate, integer, decimal)

        sign: Literal["+", "-"] = "+"

        if coordinate[0] in ("+", "-"):
            sign = coordinate[0]  # type: ignore[assignment]
            coordinate_str = coordinate[1:]
        else:
            coordinate_str = str(coordinate)

        if len(coordinate_str) > (integer + decimal):
            self.on_too_long_coordinate(self, coordinate, integer, decimal)

        if len(coordinate_str) <= 0:
            self.on_too_short_coordinate(self, coordinate, integer, decimal)

        return sign, coordinate_str

This is bare minimum to keep current behavior, so after doing that we would also have to add following policies:

• truncate_front
• truncate_back
• pad_front
• pad_back

And fine tune the algorithm for those policies to work correctly.
Each policy requires dedicate unit test and each call to policy has to be covered too.

Now to enable parametrization of policies you will have to add similar policy parameters too StateTrackingVisitor __init__ method, store them in atributes and forward them to CoordinateFormat upon creation. StateTrackingVisitor is not used directly, but rather is a base class for Compiler class so you will have to add same parameters to its constructor too. Since API was already estabilished, those parameters must be keyword only and they must have default values.
Those policy functions you have implemented must be re-exported in src/pygerber/gerber/compiler/__init__.py to make them accessible in high level API and they have to be added to compile() function in the same file.

Forwarding of those parameters requires additional unit test to confirm it works and guard against regressions in the future.

Unit tests

As for unit tests, in PyGerber we are using PyTest framework. Unit test structure mirrors this of source code, at least most of the time. Therefore if you are introducing changes in src/pygerber/gerber/ast/state_tracking_visitor.py your unit tests should end up in test/unit/test_gerber/test_ast/test_state_tracking_visitor.py (paths relative to repository root).

Please feel free to ask questions if you need more help.