Yes I realize that Kicad is written in C++, and hence the Python API is a layer in front of the C++ API. My point was just that the Python interface could have the machinery to avoid requiring casting of the Python objects returned by Duplicate().
Thanks for your other comments. They narrow down the alternatives I need to experiment with to figure out what works, or the ambiguities to which to ascribe unexpected results :-).
And it does. It casts them for you.
Duplicate() etc almost does the right thing, but I think I’m short one clue.
Basically, after footprint.Add()-ing FP_TEXT and FP_SHAPE items created by Duplicate(), the data is present in PCBNew, but does not become visible unless I save the file and then reload it. PCBNew’s Refresh function does not fix this.
Details:
I’m using the following segment of code:
# "Move" Value to selected annotation layer
fp_value_PF_TEXT = fp.Value()
fp_value_PF_TEXT.SetLayer(layer_for_annot)
[...]
# Gather copy-candidate items to a list
for gritem in fp.GraphicalItems():
lyr = gritem.GetLayer()
if lyr == pcbnew.F_Fab:
gritem_list.append(gritem)
# Duplicate listed items to annotation layer
for gritem in gritem_list:
newitem = gritem.Duplicate()
newitem.SetLayer(layer_for_annot)
fp.Add(newitem)
I’m running this from the Tools > External Plugins menu.
For what it’s worth, layer_for_annot is the constant for User.2, or 3, or 4.
After running, I observe the following in PCBNew:
So I appear to be lacking a command that will prompt PCBNew to do what’s necessary to render the changes “live”
Thanks for any suggestions!.
@MitjaN can assist you more here as his plugins deal more with editing the board than what I usually do. But I wouldn’t be surprised if pcbnew is just not updating all of it’s internal data correctly after plugin runs. You can try removing and readding the footprint that you just modified.
Yeah the KiCad’s backend code is not really taking into account what action plugin can/could do. There are some cases which are not supported. And I think that Duplicate() results should not refresh in the viewport (issue connected with this). And in my case (Save/Restore layout plugin) I’ve always used Add() and it was always for zones, tracks, text and drawing items added directly to board. I’ve never dealt with text added to footprint. So I’d try what Qu1ck is proposing.
Thanks again for your suggestions @qu1ck and @MitjaN. Still no success, but more clues:
Recap: In all cases the goal is to get a FOOTPRINT’s added FP_TEXT and FP_SHAPE items to display visually in PCBNew’s layout view, following running the External Plugin. In general:
New observations:
In the layout window, when one or more components are selected, PCBNew displays more layers and changes some texts to white (as a way of highlighting them). For a modified part, that causes the added FP_TEXTs to display (in white). They disappear again on deselecting the component.
I tried removing and re-adding the modified footprint, as you guys suggested, but that did not prompt PCBNew to show the modifications.
Same, but with a time.sleep(5) between Remove and Add (thinking that maybe there was some event that needed to happen). No improvement.
I previously noted that changing the layer of one of the required FP_TEXTs (Value) does get updated in the layout view. So maybe the problem concerns the “non-required” items accessible only via footprint.GraphicalItems()? But…
Library footprints come with a “non-required” FP_TEXT “$(REFERENCE)” for the RefId in the F.Fab layer. That’s accessible via GraphicalItems(). Using the plugin to change the layer of that item becomes visible immediately. So it’s not the GraphicalItems() membership per se that’s the issue.
Thinking that maybe Duplicate was the key factor, I tried creating a FP_TEXT from scratch (not fully knowing how many attributes I would need to set for a working new FP_TEXT):
newitem = pcbnew.FP_TEXT(fp)
newitem.SetText("TestTest")
newitem.SetPosition(fp_refid_posn)
newitem.SetLayer(layer_for_annot)
newitem.SetVisible(True)
fp.Add(newitem)
Once again, this new FP_TEXT does not become visible in the layout window, but as with the previous cases, the text does appear when the component is selected (and disappears on deselect) and also appears in the component’s Properties dialog, and if the board is saved and reloaded, the new FP_TEXT is now visible.
Tentative conclusion
In fp.Add-ing new or duplicated FP_xxx’s, there seems to be some per-FP_xxx flag that’s not getting set, or some list that the new FP_xxx’s are not getting added to. Apparently saving and reloading works around that problem. But it sure would be nice to get this to work live!
I looked at the source and it is like I suspected: it doesn’t pick up changes within footprints properly. All you have to do is replace the footprint with a new one with your changes, trick is that it has to be a new instance that you add, not the same one you removed.
So: duplicate, edit, add new one, remove old one.
Wow, thanks for your research on this. I will try that a little later this evening.
I must say that having to remove and replace all the footprints on a PCB feels a bit brittle, but it that’s the path forward I may have to add some kind of check to confirm the process completed.
Thanks for the detailed report. I’d recommend that you raise an issue on GitLab for this case. Maybe the issue gets resolved but, even if it doesn’t at least the lead developers will become aware what the plugin authors require of the python API.
Thanks
Not at all! I thank you and qu1ck for your detailed help so I could make some progress!
raise an issue on Github
Github or Gitlab? I have indeed created issue:
PCBNew API: Adding FP_TEXTs of FP_SHAPEs to existing footprints fails to update display
Thanks again!
Further update on the Duplicate() function:
I have come to the conclusion that newitem = item.Duplicate() creates a copy that is in some way dangerously shallow. That is to say, the newitem is a duplicate of the original item, but it appears to have copies of pointers to subsidiary objects that it now shares with the original.
So, for example, something like:
item = fp.Reference()
newitem = item.Duplicate()
fp.Add(newitem)
newitem.SetText("ABCD")
results in both the newitem and also the Reference() field both showing “ABCD”.
From the Python API, it’s not clear how this happens, though of course a dive into the actual C++ data structure behind (Reference()) and the Duplicate() code would reveal all.
Given this issue, the Python API doesn’t give any better confidence on any of the other “properties” that have Get and Set functions, as who knows what data structure and implementation is behind them.
My tentative conclusion is that Duplicate() on FP_TEXT (and similar) is not really usable for the purposes that its name suggests.
So instead you have to create, in effect, your own Duplicate function, which requires knowing what fields you will have to set in order to have a functioning FP_TEXT (or similar object).
So far, I’ve identified the following fields as necessary (in brief note form):
newitem = pcbnew.FP_TEXT(footprint)
SetText(newstr)
...
Get/Set Position()
Get/Set TextAngle()
Get/Set Layer()
Get/Set Visible()
Get/Set TextHeight()
Get/Set TextWidth()
Get/Set TextThickness()
Get/Set HorizJustify()
Get/Set VertJustify ()
footprint.Add(newitem)
… your Duplicate function could copy any or all of the fields from the existing item, or selectively set new values as suits your application.
Thanks for the report. I really appreciate it.
The footprint class also behaves strangely when using SetBrightened() and ClearBrightened() methods in order to highlight the footprint in drawing canvas. While both methods work as intended on tracks, zones, free text and drawing items, but fails on footprints. In order to highlight footprint one has to manually highlight all the footprint pads, text and drawing items (ReplicateLayout/replicate_layout.py at 6c91224463db883810d2f9a56ab85936121609cf · MitjaNemec/ReplicateLayout · GitHub).
As for the Duplicate() method, I am only using it on tracks, zones and free text and drawing items without any issues.
[Later Edit: The observations below were made on Kicad 6.0.7, and with less than full understanding of what aspects of the layout window PDBNew does or does not update following plugin changes to data. Later testing with PCBNew 6.0.10 did not show these same issues, or I simply misinterpreted results previously.]
While we’re on the topic of gotchas in this area, here’s one involving Position:
You might think you can do this to FP_TEXT items (here, olditem and newitem):
posn = olditem.GetPosition()
posn.x += offsetx
posn.y += offsety
newitem.SetPosition(posn)
… but what happens is that the code here modifies the position of olditem, and then gives newitem the same position.
What appears to happen here is that posn is the actual wxPoint object attached to olditem. Ie: GetPosition appears to return-value-by-reference rather than return-by-value, and SetPosition’s arguments may pass-by-reference also.
Instead you have to create a new wxPoint object:
newpos = pcbnew.wxPoint(posn.x, posn.y)
newpos.x += offsetx
newpos.y += offsety
gritem.SetPosition(newpos)
If my observations here are accurate, I think it’s a bit surprising that modifying posn changes the position of the olditem, given that olditem has an explicit SetPosition(wxPoint), which suggests that SetPosition is required to change the position.
Maybe this same kind of issue occurs throughout Kicad’s API, and I don’t know if it’s “as designed”, or a surprise outcome of firing SWIG at the C++ API.
I’ve experienced this also. I assume it is due to how the API is generated. API just exposes C++ code without much consideration. And in this case I assume that posn is not a new object but just a reference to an existing object. Thus the issue is resolved when you explicitly create a new object which you modify.
Yes I agree with your inferences.
It would be interesting to know if SWIG has some settings to control whether it generates code to pass a reference, or to create a copy and pass a value.
Like I mentioned, it seems quite counterintuitive that you can modify an item’s position either by p = GetPosition() and then fiddling with p.x and p.y, or by using SetPosition().
I share your sentiment, but for the time being I think that with developer resources available to work on python API I am afraid we’re left on our own.
So for the this specific issue I think I found out the reason for this. If you look at how the position for PCB_TEXT::GetPosition() is derived you’ll see that it just takes what EDA_TEXT::GetTextPos() returns.
Looking further how EDA_TEXT::GetTextPos() is defined, you’ll notice it returns a reference ("&" character in the return value definition)
const wxPoint& GetTextPos() const { return m_e.pos; }
The upside of this is, that I’ll get familiar with C++ and sometime maybe I’ll even be able to contribute to the KiCad’s C++ code
It’s not counterintuitive to me. Swig or not, cpp or python, nowhere is it assumed that getters return a copy of an object. When you modify internals of a position object that will reflect on the drawing that uses that object as it’s position, makes perfect sense to me.
That’s a standard programming gotcha that you ran into, not specific to KiCad at all.
I am afraid we’re left on our own
I suspect so.
Your inspection of the C++ code supports our observations. However:
virtual wxPoint GetPosition() const override
shows that GetPosition is supposed to return a wxPoint object value, not a wxPoint& reference.
So I’m a bit puzzled how we seem to be getting a reference from GetPosition.
Yeah I am still getting familiar with C++. From from what I can gather, the PCB_TEXT::GetPosition()overloads BOARD_ITEM::GetPosition(), but the function returns what EDA_TEXT::GetTextPos()returns. And EDA_TEXT::GetTextPos() returns a reference. And then this reference is returned as a value. What happens in this case I don’t have a clue. I was assuming that it returns a reference that it got.
While the observed effect might have a roots in C++ code, it might as well be caused by python or SWIG.
As python is no stranger to issues with references:
a = [1, 2, 3]
b = [a, 4, 5, 6]
print(b)
>>> [[1, 2, 3], 4, 5, 6]
a[1] = 9
print(b)
>>> [1, 9, 3], 4, 5, 6]
It might also be an issue with just FP_TEXT. I did a quick test getting a possition of a footprint and a free text item and changing a a position that I got by calling GetPosition() and I did not observe what you got
import pcbnew
import wx
brd = pcbnew.GetBoard()
fp = brd.FindFootprintByReference('Q301')
fp.GetPosition()
wxPoint(69850000, 76200000)
pos = fp.GetPosition()
pos[0] = 0
pos
>>>wxPoint(0, 76200000)
fp.GetPosition()
>>>wxPoint(69850000, 76200000)
text_items = []
for t_i in brd.GetDrawings():
if isinstance(t_i, pcbnew.PCB_TEXT):
# text items are handled separately
text_items.append(t_i)
text_item = text_items[0]
tp = text_item.GetPosition()
tp
>>>wxPoint(243062295, 79130460)
tp[0] = 0
tp
>>>wxPoint(0, 79130460)
text_item.GetPosition()
>>>wxPoint(243062295, 79130460)