tracespace / gerber-to-svg

gerber-to-svg development moved to tracespace/tracespace
https://github.com/tracespace/tracespace
MIT License
81 stars 17 forks source link

FIX: treat "no zero suppression" as "suppress tailing zero" #20

Closed summivox closed 9 years ago

summivox commented 9 years ago

Although this is not specified in the gerber file format specs, Altium does offer an option to generate gerber with no zero suppression (i.e. missing p[2]), which I always use to avoid problem with fab. This patch makes gerber parser accept these files.

mcous commented 9 years ago

Nice catch! A few things with this PR:

summivox commented 9 years ago

Can you show me what the format block %FS...*% looks like for one of these Gerber files?

%FSAX24Y24*%
%MOIN*%
G70*
G01*
G75*
G04 Layer_Physical_Order=2*
G04 Layer_Color=16711680*
%ADD10R,0.0354X0.0276*%
%ADD11R,0.0354X0.0315*%
%ADD12R,0.0709X0.0787*%

This PR will cause issues with a format spec that has a non-standard zero omission format specified rather than missing e.g. if the block is %FSNAX34Y34*%

I just added minimum code to make it work for me. I think the correct solution is to regexp it.

Before the most recent release of the Gerber Specification, it was customary to specify leading zero omission (rather than trailing) if there was no zero omission. In the latest release, trailing zero omission has been deprecated, so we should default to "leading"

You're absolutely right; I never used trailing zero suppression for exactly the same reason; must be mad when I patched.

However this ends up being implemented, can you make sure there is a unit test or two to ensure the functionality works and isn't broken in the future?

I should have but I haven't got a chance to look at your test yet.


Anyway I'll start from modifying the index-based parser.

summivox commented 9 years ago

Done, although I didn't really add a lot of tests.

mcous commented 9 years ago

Good stuff. The tests look good, but I'd still like the application to warn the user. The current implementation of the library uses console.warn (which is a terrible way to do it and in the next version is replaced with an EventEmittter that emits "warning" events, but I'm still trying to find the time to work on it).

Take a look at the drill parser for how that warning is handled and the drill parser test for how the warning message is tested for.

summivox commented 9 years ago

Travis hiccup?! I did the tests locally...


Ideally we could have the gerber-side and SVG-side decoupled in a compiler front/back fashion, and have a common interface for warnings in the front end. That'd be a major refactor/rewrite though so no expectations ;)

mcous commented 9 years ago

Cool, and yeah don't know what's up with that Travis error. I will try to get this merged tonight.

If you're curious, I just pushed the rewrite I'd been working on to the streaming-reader branch. I only just got finished with the plotter, so anything past that (converting to an XML string, using the CLI) isn't done yet.

I'd like further decoupling, so that the plotter generates a more abstract format that could lend itself well to analysis, but that's a further project. Plus right now, the rewrite is about three times slower (at worst case) that the current library, so that's not great.

mcous commented 9 years ago

Merged and released as 0.4.2. Thanks for the PR!