Add footprint Parameter to Kicad Footprint mapping #44

30350n · 2023-11-18T16:14:56Z

Okay, here's another feature that probably requires some discussion though 😅

The idea is to add a footprint_parameter_mapping field, which acts as a dictionary which maps values from the footprint parameter field to actual KiCad footprint names. I like this, because it enables me to have a "Package Type" field which is for example set to values like 0805, 0603, etc. (for resistors) which can then be mapped to Resistor_SMD:R_0805_2012Metric, Resistor_SMD:R_0603_1608Metric, etc.

In my case this is even more useful, because I don't have to set the Package Type parameter manually, but it gets automatically set by my InvenTree Part Import CLI.

What I don't like about this current implementation I have, is the JSONField really ... It's not really that nice to use.

afkiwers · 2023-11-18T22:26:24Z

@30350n would you be able to prove a working example? And why a JSON field?

Also, if we merge this I think this should be an optional which means we would need a switch; and probable another setting which allows you to define which parameter should used as some people might not like 'Package Type' and go rather with something else.

Otherwise, I do like the idea.

30350n · 2023-11-18T22:46:34Z

would you be able to prove a working example

Sure!

Here's one of my resistors with a "Package Type" parameter set to "0603 (1608 Metric)".

Here's the KiCad Category for my Resistors (the bottom two fields are new):

And here's what your API returns:

another setting which allows you to define which parameter should used

This already exists in the form of your Footprint Parameter setting. + I added another field to the KiCad Category which lets you overwrite said setting per category (because I think that can be very useful, I don't actually need it right now).

And why a JSON field?

The JSONField is more or less the part I'm unsure about. It just seemed like the quickest way to add "dictionary" like data to a django model. I'd also prefer something that would be easier to setup and wouldn't require you to write out an actual dictionary in JSON.

I think this should be an optional which means we would need a switch

It already is optional more or less. If there is not match for a value in the JSON dictionary, it will simply be ignored:

footprint = footprint_mapping.get(footprint, footprint)

(from here)

afkiwers · 2023-11-20T06:04:09Z

Instead of JSON one could use a new model which holds the template footprints and refer to them from the category using one-to-many rel.

This way one could use _contains in an SQL query to find the matching one.

Also, takes away the complexity of writing syntax correct JSON.

However, I'm unsure if just adding the correct footprint to each part might be less failure prone.

@SchrodingersGat any thoughts on that?

30350n · 2023-11-20T08:04:17Z

Instead of JSON one could use a new model which holds the template footprints and refer to them from the category using one-to-many rel.
This way one could use _contains in an SQL query to find the matching one.

Yeah okay, that's kind of what I figured, but I was unsure how that'd look like in the Admin view, I'll try that, thanks!

However, I'm unsure if just adding the correct footprint to each part might be less failure prone.

Well like I said ... this change doesn't disable that. It just makes it easier for people who already have some kind of footprint parameter setup to use this plugin. Without it, one would have to add "the correct" footprint for all the parts manually.

afkiwers · 2023-11-20T09:56:39Z

If you can implement it so it's easy to use and understand I'm happy to merge it.

We should probably also start thinking about some sort of documentation to help people understand what this plugin does.

SchrodingersGat · 2023-11-21T02:13:35Z

We should probably also start thinking about some sort of documentation to help people understand what this plugin does.

This feature in particular is pretty esoteric, so some clear documentation around how it works would be great.

afkiwers · 2023-11-21T04:31:54Z

We should probably also start thinking about some sort of documentation to help people understand what this plugin does.

This feature in particular is pretty esoteric, so some clear documentation around how it works would be great.

So let's merge it as is?

@30350n would you be able to add a quick description to the README.md file?

Once that's done, let's just merge this version. This seems to be the easiest way at the moment, given other ones would require new models.

30350n · 2023-11-22T01:56:44Z

This feature in particular is pretty esoteric, so some clear documentation around how it works would be great.

Esoteric, but very useful imo 😅

So let's merge it as is?

I think I wanna try the other way via an extra model first. If we'd ship this as is and decide to change it later on, migrations are going to be hell. Better to just implement the better solution in the first place ^^

afkiwers · 2023-12-05T22:54:05Z

@30350n how did you go with the models instead of json?

30350n · 2023-12-07T13:37:05Z

@30350n how did you go with the models instead of json?

Didn't really have time to look into this yet ^^

afkiwers · 2023-12-07T14:25:53Z

I'm happy to merge this approach. This way we don't have to worry about migrations etc.

30350n · 2023-12-07T14:30:12Z

I would really rather not. While it's generally working, it's really not userfriendly at all, especially because you have to manually edit the json everytime you want to make a change.

afkiwers · 2023-12-07T14:45:24Z

Understood

Nudelsalad · 2023-12-11T11:39:24Z

This is what people want!
I would be more than fine with the json input field but why not add two text fields and map one mapping after another by pressing + or add fields

afkiwers · 2023-12-12T09:55:53Z

As I mentioned before, I'm happy to merge this version if @30350n is happy with this work.

30350n · 2023-12-13T00:35:16Z

Can't tell me this isn't miles better in terms of useability ^^

And it's also still working as it should ofc.

30350n · 2023-12-13T00:35:37Z

Ready for rereview/merging now :)

30350n · 2023-12-13T01:04:42Z

Also just added a quick paragraph to the README and fixed it's formatting.

afkiwers · 2023-12-13T16:14:33Z

I'll run it on my dev setup and merge it afterwards if there are no issues.

Thanks @30350n

afkiwers · 2023-12-14T22:55:06Z

@30350n I just ran your code and added some suggestions and questions

also, would you mind baselining your code so it current. Currently it's not mergeable.

30350n · 2023-12-15T01:36:25Z

@30350n I just ran your code and added some suggestions and questions

Uhhh, where exactly? Can't seem to find anything...

also, would you mind baselining your code so it current. Currently it's not mergeable.

Will rebase tomorrow.

inventree_kicad/models.py

afkiwers · 2023-12-14T22:28:33Z

inventree_kicad/models.py

@@ -51,6 +51,35 @@ class Meta:
        help_text=_('Default value parameter template for this category, if not specified for an individual part'),
    )

+    footprint_parameter_template = models.ForeignKey(


What was your intention with that extra parameter? It confused me a fair bit initially.

That's just the existing KICAD_FOOTPRINT_PARAMETER setting, but on a per KiCad category level.

I'm uncertain about this particular case. We already have the settings parameter in place for that purpose. I can't envision a scenario where the footprint would be stored across multiple parameters instead of just one. It seems like that could become confusing quite rapidly.

Well pretty much exactly for the reason this whole PR exists.

In my case for example, I've got a whole bunch of components in InvenTree already. Some of the (resistors, capacitors, etc.) already have a "Package Type" parameter set, which I can use with the introduced functionality here, to map from said "Package Type" to a KiCad Footprint. But that "Package Type" parameter isn't set for all parts. So for some more specialized parts, like Potentiometers or certain ICs or whatever, I'd prefer to actually just add a "KiCad Footprint" parameter directly, which I can change from the InvenTree UI.

So what I'd do is set the global KICAD_FOOTPRINT_PARAMETER setting to the "KiCad Footprint" parameter and then override it for certain categories (resistors and capacitors for example) to the "Package Type" parameter.

Nothing confusing about that really, it's just a simple override.

Yeah okay that makes sense now.

Would you be able to add a bit more information to the readme about when you would use which way. There are pretty much 3 then now.

Maybe include an example of some sort.

Cheers

afkiwers · 2023-12-15T02:22:33Z

Uhhh, where exactly? Can't seem to find anything...

My bad, I didn't press submit. See above! :-)

This reverts commit 2249ac8.

afkiwers · 2023-12-16T07:37:22Z

inventree_kicad/migrations/0006_auto_20231212_1720.py

migration file seems outdated. Did you rebase? I added a new model.

Ah, I see, missed that. Why would you need a model for a progress indicator btw? 🤔

Multiple users import stuff at the same time. Also multi-file upload in the future maybe.

I had crosstalk without it.

Interesting. Still seems like something that should just be stored in a variable, not the database.

Yes so I thought initially

afkiwers · 2023-12-16T07:41:15Z

inventree_kicad/models.py

@@ -51,6 +51,35 @@ class Meta:
        help_text=_('Default value parameter template for this category, if not specified for an individual part'),
    )

+    footprint_parameter_template = models.ForeignKey(


I'm uncertain about this particular case. We already have the settings parameter in place for that purpose. I can't envision a scenario where the footprint would be stored across multiple parameters instead of just one. It seems like that could become confusing quite rapidly.

30350n · 2023-12-16T22:08:34Z

Thanks for merging! :)

afkiwers · 2023-12-16T22:12:01Z

Thanks for taking your time to make this plugin better.

afkiwers · 2023-12-27T22:25:41Z

I just noticed it created an additional migration file on my production server but not in my dev server (_008). Have you seen similar behaviour?

30350n · 2023-12-27T22:59:33Z

Did you migrate with a previous version of this PR on it before?

afkiwers · 2023-12-31T05:27:49Z

it's a bit odd. I checked the containers and there is no additional migration file.

However, the database says there's one.

afkiwers · 2023-12-31T05:50:41Z

I believe I found the issue. It looks like you changed the help_text and forgot to execute makemigrations.

afkiwers · 2023-12-31T05:59:45Z

Addressed in #66

30350n · 2023-12-31T09:24:09Z

Ah! That makes sense, sorry about that ^^

afkiwers · 2023-12-31T09:36:15Z

No Biggi! It just confused me a lot.

Implement footprint_parameter to kicad footprint mapping

af11704

30350n changed the title ~~Add footprintparameter to kicad footprint mapping~~ Add footprint Parameter to Kicad Footprint mapping Nov 18, 2023

Revert bad refactoring

9d860eb

30350n mentioned this pull request Dec 7, 2023

[FEATURE[S]] Add Datasheet and HTTP-LIB Support 30350n/inventree_part_import#1

Closed

30350n added 3 commits December 12, 2023 18:03

Reimplement footprint mapping via new Model, Revert old migration

b5a7c92

Fix migration, formatting

d7febf0

Fix migration again

ad70e1d

30350n added 6 commits December 13, 2023 01:46

Add documentation

1340bbd

Add image for documentation

caccf0f

Merge remote-tracking branch 'origin/main' into footprint-mapping

d2e4dfb

Fix README formatting (trailing spaces, headings, image links)

3602d19

Limit README to 96 columns

2249ac8

Fix png case

33239f2

afkiwers reviewed Dec 15, 2023

View reviewed changes

30350n added 4 commits December 15, 2023 14:10

Revert "Limit README to 96 columns"

1e30c11

This reverts commit 2249ac8.

Merge remote-tracking branch 'origin/main' into footprint-mapping

2486e6e

Fix formatting, Add __str__ function to mapping model

291bfa1

Fix README formatting

049d86c

afkiwers reviewed Dec 16, 2023

View reviewed changes

30350n added 2 commits December 16, 2023 15:41

Update migration

f95e882

Add footprint parameter override example, clarify help message

fc47cf4

afkiwers merged commit 62d5c58 into afkiwers:main Dec 16, 2023
1 check passed

afkiwers mentioned this pull request Dec 16, 2023

Format README with column limit #65

Closed

afkiwers mentioned this pull request Dec 31, 2023

Migration file missing #66

Closed

Add footprint Parameter to Kicad Footprint mapping #44

Add footprint Parameter to Kicad Footprint mapping #44

Conversation

30350n commented Nov 18, 2023 • edited Loading

afkiwers commented Nov 18, 2023

30350n commented Nov 18, 2023 • edited Loading

afkiwers commented Nov 20, 2023 • edited Loading

30350n commented Nov 20, 2023

afkiwers commented Nov 20, 2023

SchrodingersGat commented Nov 21, 2023

afkiwers commented Nov 21, 2023

30350n commented Nov 22, 2023

afkiwers commented Dec 5, 2023

30350n commented Dec 7, 2023

afkiwers commented Dec 7, 2023

30350n commented Dec 7, 2023 • edited Loading

afkiwers commented Dec 7, 2023

Nudelsalad commented Dec 11, 2023

afkiwers commented Dec 12, 2023 • edited Loading

30350n commented Dec 13, 2023

30350n commented Dec 13, 2023

30350n commented Dec 13, 2023

afkiwers commented Dec 13, 2023

afkiwers commented Dec 14, 2023

30350n commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afkiwers commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

30350n commented Dec 16, 2023

afkiwers commented Dec 16, 2023

afkiwers commented Dec 27, 2023

30350n commented Dec 27, 2023

afkiwers commented Dec 31, 2023

afkiwers commented Dec 31, 2023

afkiwers commented Dec 31, 2023 • edited Loading

30350n commented Dec 31, 2023

afkiwers commented Dec 31, 2023

30350n commented Nov 18, 2023 •

edited

Loading

30350n commented Nov 18, 2023 •

edited

Loading

afkiwers commented Nov 20, 2023 •

edited

Loading

30350n commented Dec 7, 2023 •

edited

Loading

afkiwers commented Dec 12, 2023 •

edited

Loading

afkiwers commented Dec 31, 2023 •

edited

Loading