fritzing / fritzing-app

Fritzing desktop application
http://fritzing.org
Other
4.04k stars 835 forks source link

Changing the type in the fzp file (either way) causes the obsoleting mechamism to misfunction. #3979

Open vanepp opened 2 years ago

vanepp commented 2 years ago

Current Behaviour

Using these two test .fzz files (remove the trailing .zip to get the .fzz)

Dual_VNH2SP30_Motor_Driver.fzz.zip

Arduino-Mini-v5.fzz.zip

owner@owner-PC /cygdrive/d/repos/fritzing-parts $ git checkout -b Arduino-Mini-v5 Switched to a new branch 'Arduino-Mini-v5'

$ obsolete.py core/Arduino-Mini-v5.fzp Arduino-Mini-v5 ('git', 'mv', 'core/Arduino-Mini-v5.fzp', 'obsolete/Arduino-Mini-v5.fzp') Set orig version to "5" replace moduleId="Arduino-Mini-v5" with moduleId="Arduino-80fc344b07c9495f94f4d8b5f91df439" set new version to "6" set obsolete_fzp family to "//obsolete//microcontroller board (arduino)" image "breadboard/Arduino-Mini-v5_breadboard.svg" path "svg/core/breadboard/Arduino-Mini-v5_breadboard.svg" ('cp', 'svg/core/breadboard/Arduino-Mini-v5_breadboard.svg', 'svg/core/breadboard/Arduino-Mini-v5_3981dbf_6_breadboard.svg') ('git', 'mv', 'svg/core/breadboard/Arduino-Mini-v5_breadboard.svg', 'svg/obsolete/breadboard/Arduino-Mini-v5_breadboard.svg') ('git', 'add', 'svg/core/breadboard/Arduino-Mini-v5_3981dbf_6_breadboard.svg') set layer image to breadboard/Arduino-Mini-v5_3981dbf_6_breadboard.svg image "schematic/Arduino-Mini-v5_schematic.svg" path "svg/core/schematic/Arduino-Mini-v5_schematic.svg" ('cp', 'svg/core/schematic/Arduino-Mini-v5_schematic.svg', 'svg/core/schematic/Arduino-Mini-v5_3981dbf_6_schematic.svg') ('git', 'mv', 'svg/core/schematic/Arduino-Mini-v5_schematic.svg', 'svg/obsolete/schematic/Arduino-Mini-v5_schematic.svg') ('git', 'add', 'svg/core/schematic/Arduino-Mini-v5_3981dbf_6_schematic.svg') set layer image to schematic/Arduino-Mini-v5_3981dbf_6_schematic.svg image "pcb/Arduino-Mini-v5_pcb.svg" path "svg/core/pcb/Arduino-Mini-v5_pcb.svg" ('cp', 'svg/core/pcb/Arduino-Mini-v5_pcb.svg', 'svg/core/pcb/Arduino-Mini-v5_3981dbf_6_pcb.svg') ('git', 'mv', 'svg/core/pcb/Arduino-Mini-v5_pcb.svg', 'svg/obsolete/pcb/Arduino-Mini-v5_pcb.svg') ('git', 'add', 'svg/core/pcb/Arduino-Mini-v5_3981dbf_6_pcb.svg') set layer image to pcb/Arduino-Mini-v5_3981dbf_6_pcb.svg image "breadboard/Arduino-Mini-v5_breadboard.svg" path "svg/core/breadboard/Arduino-Mini-v5_breadboard.svg" set dup layer image to breadboard/Arduino-Mini-v5_3981dbf_6_breadboard.svg warning: moduleId="Arduino-Mini-v5" not found in bins/core.fzb, core.fzb not updated. Write core/Arduino-Mini-v5_3981dbf_6.fzp Write obsolete/Arduino-Mini-v5.fzp ('git', 'add', 'core/Arduino-Mini-v5_3981dbf_6.fzp') ('git', 'add', 'obsolete/Arduino-Mini-v5.fzp')

$ vi core/Arduino-Mini-v5_3981dbf_6.fzp

:1,$s/type="male"/type="female"

then save and commit the file.

$ git commit -m "Change type=male to type=female for testing" [Arduino-Mini-v5 b4843db2] Change type=male to type=female for testing 9 files changed, 1536 insertions(+), 431 deletions(-) delete mode 100644 core/Arduino-Mini-v5.fzp create mode 100644 core/Arduino-Mini-v5_3981dbf_6.fzp create mode 100644 obsolete/Arduino-Mini-v5.fzp rename svg/core/breadboard/{Arduino-Mini-v5_breadboard.svg => Arduino-Mini-v5_3981dbf_6_breadboard.svg} (100%) rename svg/core/pcb/{Arduino-Mini-v5_pcb.svg => Arduino-Mini-v5_3981dbf_6_pcb.svg} (100%) mode change 100755 => 100644 rename svg/core/schematic/{Arduino-Mini-v5_schematic.svg => Arduino-Mini-v5_3981dbf_6_schematic.svg} (100%) create mode 100644 svg/obsolete/breadboard/Arduino-Mini-v5_breadboard.svg create mode 100755 svg/obsolete/pcb/Arduino-Mini-v5_pcb.svg create mode 100644 svg/obsolete/schematic/Arduino-Mini-v5_schematic.svg

copy the new parts repo in to Fritzing then run (as administtator on windows!)

Parts->regenerate parts database

to update the parts db with the new part.

load Arduino-Mini-v5.fzz

click "no" to update part to get unchanged part. Which produces this:

pr1

pr2

pr3

now exit Fritzing without saving and reload Arduino-Mini-v5.fzz except this time say yes to upgrade which produces this:

pr4

pr5

pr6

where the upgrade has failed (with the only change being connector type from male to female)

use the same steps but with Dual_VNH2SP30_Motor_Driver.fzz to demonstrate that the change either way breaks:

pr10

pr11

pr12

pr13

Until this bug is fixed a workaround is to do a two stage upgrade. First do the change of type from male to female or female to male without doing an obsolete. That should work correctly and should not need obsoleting, once that is committed then you should be able to do the changes you want and successfully obsolete the part.

Build:

Version 0.9.10 (bCD-2134-0-40d23c29 2022-07-01) 64 [Qt 5.15.2]

Operating System:

Windows 10

Steps to reproduce:

See current behaviour

Expected Behaviour

The part should update and connect correctly.

KjellMorgenstern commented 2 years ago

What does "connect correctly" mean if you change the gender of the connectors? Those are not arbitrary.

Maybe it would be better to have two variants of the part, one with male and one with female headers soldered on?

In any case, this example shows again that the update mechanism for new versions of parts should be improved (e.g show an error, fallback to the type of reconnection that probably was intended here) . Besides other things, like guiding the user through replacing the updates parts step by step.

vanepp commented 2 years ago

"What does "connect correctly" mean if you change the gender of the connectors? Those are not arbitrary."

What I expected to happen is the change should just work because I think it will just work if you make the same change without doing the obsolete. Changing the type of the connector should not (and doesn't seem to) affect anything if done outside the obsoleting mechanism. In this particular case I believe the female type connectors are incorrect, there isn't any reason for them to not be male as the board won't fit on a breadboard (which is the usual reason for female connectors to prevent a short if attached to a breadboard) and it should be (at least in my view) be possible to change them as part of the obsoleting process. As noted if this is to hard to fix easily, it can be done by making the change from female to male without obsoleting the part, commit that, then do the obsolete with the connectors in the desired configuration, it is just an extra step that I would like to avoid if possible.

vanepp commented 1 year ago

@mMerlin pointed out in the forums that my work around (do an update without obsoleting for just the type, then once that is committed do another update that obsoletes) in fact won't always work. If you do that, old sketches (which have the previous type in them) will try and update the type and exhibit the error. For now I will leave the type set to "female"in the motor driver part. I suspect the hard part here is that when obsoleting is done, the sketch is already loaded and thus changing the type is much harder than when you you are loading the part from a file (as it has no type set by default and will use whatever the file says.) This may be a rare enough thing to want to do that we could ignore it, but if someone changes the type as part of an obsolete it will break. One thing that should work (but is ugly!) would be to change the type in the obsolete .fzp file to the new type. That way when the part loads it will have the new type and the obsolete will work correctly. We could modify the obsolete.py script to do this if we decide it is an acceptable workaround. The downside is that we are changing a parameter (which should be usually safe, but there may be cases where it isn't!) in the original part file which may affect older sketches when the type changes on load. I don't see any particularly easy solution to this!