search

opulo-inc / lumenpnp

The LumenPnP is an open source pick and place machine.
Other
2.42k stars 323 forks source link

[Release assets] Position files are missing fiducial location #566

Open G-Pereira opened 2 years ago

G-Pereira commented 2 years ago

The release assets/artifacts that originate from our CI do not include fiducials on the gerber position files

MC42 commented 2 years ago

Copying the small amount of digging I'd done in Discord here:

https://github.com/opulo-inc/lumenpnp/blob/main/.github/workflows/scripts/kibot/config-4layer.kibot.yaml

This configuration is one of four in the folder used for CI/CD production of these files, including how we produce the locations for picking and placing.

https://github.com/INTI-CMNB/KiBot#built-in-filters

The README.md for KiBot includes the documentation for the application and the few references to fiducials seem to be linked to the built-in filters, and excluding specifically _mechanical parts.

As this is the default exclude filter, I suspect if we ported / updated the KiBot exclude filter to specifically include fiducials (may need to have every item in their set copied and manually removed), that'd get us further along.

elafontaine commented 2 years ago

Hi @G-Pereira ,

I've started looking into this issue.

I must admit that I need your help into understanding this one :). I'm trying to figure out what exactly you're looking for in those gerber file :). For me, fidutials only means the "scale" into the file. However, gerber files are made to be specific to certain metrics. If you could help me identify what isn't shared in the gerber files, I could resolve the issue for you :).

Could you provide me the information in regards to what you are looking for exactly ? If you do, I'll do what I can to see it done ;).

G-Pereira commented 2 years ago

Thanks @elafontaine . The problem is that the position files do not include fiducials (marks on the PCB used by the pick and place to calibrate reference position and rotation).

image

When loading position files (which describe the every component location on the PCB) these should include fiducials as well so it can be configured on the machine's software (for example OpenPnP)

elafontaine commented 2 years ago

ok, I've found some example of this :) thank you.

I've found the issue you were talking about. I'll read into the logs of the action that generated them and why kibot makes it that way. I tried to run it on my kicad software, but it always generated the FID components on the file for me... my guess is that one of the option is removing it... got to figure out which one :D.

Anyway, I'll continue on Friday. For those trying to understand the issue, download the latest release "PCB" package from https://github.com/opulo-inc/lumenpnp/releases . From there, look as the *_cpl_jlc.csv (or any "cpl" file (Component Placement)). These file are missing the FID x (where x is an integer) which would be the position of the Fiducials.

You can then open the component you looked at in KiCad and "File" > "Fabrication Output" > "Component Placement (.pos)" In there, change the output format to csv and observe the difference :D.

Stay tuned

G-Pereira commented 2 years ago

Thank you! Just one minor thing: <board>_cpl_jlc.csv and <board>_cpl_jlc_conn.csv should not have the fiducials. The ones that need to have it are <board>-bottom_pos.csv and <board>-top_pos.csv

elafontaine commented 2 years ago

Ok, started to look tonight.

Kibot default exclude filter on the "base_filter' remove the FID;

My current hypothesis is that the global setting apply the "default" filter which include the _mechanical one. I'm trying to understand why we set it like that and why we couldn't just remove that from the setting (and apply the "default" where it is needed and/or specific filter where we need something specific).

To be continued.

Question; Would we mind keeping the output of the generation of files at CI time for releases only. (Food for thoughts ?)

Output on my ubuntu server using kibot as CI does; 

- 'Pick and Place Location File' (pick_and_place_file) [position]
DEBUG:Output destination: /root/lumenpnp/pnp/pcb/mobo/mobo/gerbers (kibot - kiplot.py:361)
DEBUG:Filters reset (kibot - fil_base.py:161)
DEBUG:Applying filter `_mechanical` to exclude (kibot - fil_base.py:154)
DEBUG:Excluding 'FID1': Field 'reference' (FID1) matched 're.compile('^FID', re.IGNORECASE)' (kibot - fil_generic.py:160)
DEBUG:Excluding 'FID2': Field 'reference' (FID2) matched 're.compile('^FID', re.IGNORECASE)' (kibot - fil_generic.py:160)
DEBUG:Excluding 'FID3': Field 'reference' (FID3) matched 're.compile('^FID', re.IGNORECASE)' (kibot - fil_generic.py:160)
DEBUG:Excluding 'FID4': Field 'reference' (FID4) matched 're.compile('^FID', re.IGNORECASE)' (kibot - fil_generic.py:160)
DEBUG:Excluding 'FID5': Field 'reference' (FID5) matched 're.compile('^FID', re.IGNORECASE)' (kibot - fil_generic.py:160)
DEBUG:Excluding 'FID6': Field 'reference' (FID6) matched 're.compile('^FID', re.IGNORECASE)' (kibot - fil_generic.py:160)

When I remove the "global: variant: default" it does not do this.

elafontaine commented 2 years ago

Just after realizing this, I found the variant in our config (so not one of the available "default" variants)... I'm blind XD.

  - name: default
    comment: 'Just a place holder for the rotation filter'
    type: kibom

Default behaviour of Kibom on generic filter :

generic: Generic filter This filter is based on regular expressions. It also provides some shortcuts for common situations. Note that matches aren't case sensitive and spaces at the beginning and the end are removed. The internal _mechanical filter emulates the KiBoM behavior for default exclusions. The internal _kicost_dnp filter emulates KiCost's dnp field.

This explain why _mechanical is applied ( at least, I believe based on the explanation and log provided in the previous message).

This goes back to my initial hypothesis which I will confirm deeper later. My suggestion would be to change the variant on that task to be the real default one. In my eyes, less configs to maintain, the better. Let me know what you think.

G-Pereira commented 2 years ago

That makes sense. I would just check if that wasn't put there to trigger something else that was missing before and would change to the actual default value as you said, I wouldn't remove it. If I recall correctly @daveismith we had the approach of specifying all parameters even if they were set to their defaults for transparency and consistency concerns, correct? We did that to avoid messing our things up if KiBot decided to change their default values at some point.

daveismith commented 2 years ago

My recollection is that we went with the kibom type as it was producing the desired output format. The only filtering we were intentionally doing was to covert names / packages for JLCPCB.

elafontaine commented 2 years ago

I'll push the PR tomorrow during the day, but I ran a diff of the files between applying the variant option individually on any tasks vs using the global setting;

Over-all, it doesn't seem to affect the content. I wouldn't be able to confirm the binary file content, any leads I could follow on?

elafontaine commented 2 years ago

If I understood correctly, all you wanted is the default behaviour of kibot for position file (gerber folder). This could be easy if we remove any "variant" to be applied on it. We can have other variant in other tasks (like the global Kibom one).

Please have a look and let me know how you would wish to proceed.

elafontaine commented 2 years ago

By the way, I realised I took an assumption that this was needed on every config file (for the position file). Was that wrong ?

G-Pereira commented 2 years ago

Please share a link to the workflow run where those files are being generated and the <board>-top_pos.csv include the FID entries.

elafontaine commented 2 years ago

I'm not sure about what you're asking. You want me to send the command I ran on my ubuntu server ? or just the file that shows the csv has the FID in it ? or zip the whole gerber folder and share it ? Or do I have a way to trigger the CI ?

elafontaine commented 2 years ago

Ok, I think I found what you meant (I didn't knew that github runners were free, nor that I could just activate a "workflow").

https://github.com/elafontaine/lumenpnp/actions/runs/3338496971/jobs/5526227581

elafontaine commented 2 years ago

BTW, as I mentionned, I did modify the config file for 2, 4 and general(?) layers. Was that desired ?

G-Pereira commented 2 years ago

All good, don't worry