Skip to content

Fix holder geometry export - #11

Closed
dementive wants to merge 1 commit into
mainfrom
nathan/fix-holder-export
Closed

dementive wants to merge 1 commit into
mainfrom
nathan/fix-holder-export

Conversation

@dementive

Copy link
Copy Markdown
Collaborator

No description provided.

@JustinSGray

Copy link
Copy Markdown
Contributor

Good catch, and it's a real defect — but it can't land here, because the code this edits no longer exists on the branch this would merge into.

efaa214 ("Take the packaged Fusion exporter", open as #9) deleted app/shared/fusion-library.ts and its test and moved the export into @toolpath/tool-support/export/fusion, whose per-type rules come from Autodesk's published JSON Schema. app/shared/fusion-input.ts is the whole seam that remains. Merging this to main and then main into paul/tool_catalog is a modify/delete conflict on both files, and the resolution is to take the deletion:

CONFLICT (modify/delete): apps/catalog/app/shared/fusion-library.ts
  deleted in HEAD and modified in nathan/fix-holder-export

The bug survived the move, so the finding still stands. fusionHolder's published arm does exactly what holderOf did here — writes holder.gaugeLength against a stack that fromPublished builds out to projection. The measured arm was already right: it cuts at the gage line with belowGageLine and takes the gauge length from the last vertex.

Carried upstream as toolpath/ui-packages#114, with the reasoning and the numbers written down:

  • gaugeLength is derived from the exported stack on both arms, so a document can never again state one length while drawing another.
  • Where the vendor's figure disagrees, a dropped note names both numbers — on a BT 30, REGO-FIX's B4 - B3 is 48.4 mm, the gauge-line-to-flange distance, and that's the whole gap.
  • assemblyGaugeLength needed nothing: geometry.ts already takes the holder gauge from the fusionHolder result, so it's null when the holder is dropped. That's your second change, already correct upstream.

Both fixtures published projection === gaugeLength, which is why nothing caught this — here and upstream both. #114 adds a case where they differ; four of its five assertions fail against the old behaviour.

Closing this one as superseded. The template-side follow-up (dep bump, fixture, and the 'gauge length' warning test whose meaning changes) lands after #114 releases and #9 merges.

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.

2 participants