yaqwsx / KiKit

Automation tools for KiCAD
https://yaqwsx.github.io/KiKit
MIT License
1.49k stars 200 forks source link

Handle correction offsets and rotation separately #707

Open mikesmitty opened 1 month ago

mikesmitty commented 1 month ago

I ran into some very confusing/unintuitive behavior when setting both rotation and X/Y offsets on a component, and I think it's a bug, but please correct me if I'm wrong.

I have a PCB file where the rotation of a component is set to -90, and I noticed when uploading the position file for a PCBA job that it is off by 180 degrees from JLCPCB's library. I set the JLCPCB_CORRECTION field to 0;0;180 in my schematic to fix that, then went to update the X/Y offsets which were also offset. But because the component's rotation ended up being converted to 90 degrees this function effectively swapped X and Y, which is very confusing, especially when making changes to both X and Y at the same time.

I don't think it makes sense for the rotation angle to be taken into account when applying the X/Y correction fields here. Intuitively, I expect X and Y to be applied as is to the final positions regardless of the rotation, but let me know what you think

yaqwsx commented 1 month ago

This is a documented behavior: https://yaqwsx.github.io/KiKit/latest/fabrication/jlcpcb/#assembly

The idea is that you reference the XY correction in the footprint coordinate system, not the board one. It means that you can directly enter the difference between the origin in the component's datasheet and KiCAD footprint. It can, however, be confusing when you are trying to fine-tune position based on the board view.

I was always correcting my components based on the datasheet. Hence, this is what is implemented in KiKit.

If we are about to change this, then this is a breaking change and it needs to be part of a major release. To make such a significant change, we need to collect more opinions among the KiKit's user.