fritzing / fritzing-app

Fritzing desktop application
http://fritzing.org
Other
3.97k stars 826 forks source link

Half_breadboard.fzp is not created as a Bredboard object #3873

Closed failiz closed 3 months ago

failiz commented 3 years ago

The Half breadboard (Half_breadboard.fzp, half+ in the inspector) is being created as an object of PaletteItem instead of Bredboard. This means that this breadboard can be rotated 45 degrees and it is being greyed out by the simulator. All the other breadboards are instances of Breadboard. 0.9.8 (Windows)

Steps to reproduce:

Any idea how to fix this?

vanepp commented 3 years ago

This will likely do it (untested so far though!) in Half_breadboard.fzp change the ModuleId to have -ModuleID appended to it (this I believe is a requirement along with family Breadboard and breadboardbreadboard as the breadboard layerId to make a real breadboard. The last two are already correct leaving only the lack of the trailing ModuleID)

edit: had to add a space after the < and before the trailing > to get it to display here. I don't know how to escape xml like in the forum here!

< module fritzingVersion="0.6.0b.07.08.5201" moduleId="13f88e58b86d08d73d112f0fb05e6968" >

to

< module fritzingVersion="0.6.0b.07.08.5201" moduleId="13f88e58b86d08d73d112f0fb05e6968-ModuleID" >

everything else looks correct to me.

edit:

Changed the fzp file and regenerated the database and now it works as expected, so the change appears to work.

Peter

failiz commented 3 years ago

Yes, with that change, the breadboard is created as a Bredboard object. However, I think we need to obsolete the part and create a new one as the moduleID has changed. For example, when opening a project which has the half+board, Fritzing complains that cannot find that part if I regenerate the database modifying the moduleID.

What perplexed me is that I do not know why is not taken as a breadboard type without modifying the moduleID. In palettemodel.cpp, line 300 you can see:


else if (propertiesText.contains("breadboard", Qt::CaseInsensitive)) {
        type = ModelPart::Breadboard;

So, if there is a "breadboard" contained in the properties, it should be set as a Breadboard type, which later triggers a breadboard object in partfactort.cpp, line 190:

if (modelPart->itemType() == ModelPart::Breadboard) {
    return new Breadboard(modelPart, viewID, viewGeometry, id, itemMenu, doLabel);
}

But there is something else going on that I do not understand...

@KjellMorgenstern , could you fix the half breadboard or let me know ho to obsolete it?

KjellMorgenstern commented 3 years ago

Indeed, obsoleting a part that is probably widely used might be a problem. And if someone happens to have a circuit with a 45 degree rotation, they might not be happy if that board is replaced by a board that doesnt rotate by 45 degree anymore.

Besides the odd look, does the 45 degree rotated breadboard work as expected? Or is it broken? Maybe it is somthing we do not need to fix?

failiz commented 3 years ago

The only (small) problem is that I use the object type to decide the parts to grey out in the simulator. But we can leave this without fixing it and change the examples of the simulator to use a different breadboard. Ideally, we could make a new part and leave the old one, but not sure if there is a way to force a given part in the swapping mechanism (the new and old part would have the same properties).

vanepp commented 3 years ago

"However, I think we need to obsolete the part and create a new one as the moduleID has changed."

Yes I expect that is true. Obsoleting the current part should fix it though I believe. Sketches using the current part will use the old part unless the user agrees to update to the new one. Somewhere in the source something is checking for the appended ModuleID (although a grep through the source just now doesn't tell me where , which is not unusual.) I learned this from @SteelGoose (who unfortunately no longer posts!) in the forums 3 or 4 years ago when doing some work with breadboards. I think the best way forward is to change the half+ breadboard in core parts and then obsolete the old part. That should make the half+ part be the same as all (I think!) the other breadboards which is probably desirable.

For certain values of "works" the 45 degree rotation appears to work correctly. If the components are placed before the rotation all is well:

capture

then rotate:

capture1

but adding a new component (unless you rotate it 45 degrees as well) doesn't work:

capture2

So I think the best bet would be to obsolete the current half+ breadboard and replace it.

KjellMorgenstern commented 3 years ago

I still don't really see an issue with the 45 degree rotation. Of course it's inconvenient to rotate everything, but if that's the way you want to use it, why not.

About the greying out. Don't you use the spice property to decide about greying out? Or do you add exceptions for breadboards, as they are just treated as ideal connectors?

And yes: I agree the part should be fixed to get correct Breadboard character. But I'd prefer to have the obsoletion mechanism tested for this breadboard to avoid adding problems

failiz commented 3 years ago

"I still don't really see an issue with the 45 degree rotation. Of course it's inconvenient to rotate everything, but if that's the way you want to use it, why not." Yes, I agree with this. However, this could be change in the code. Not sure, why restricted the 45 degree rotations in breadboards. In any case, I think that the 45 degree rotation is used very little.

"About the greying out. Don't you use the spice property to decide about greying out? Or do you add exceptions for breadboards, as they are just treated as ideal connectors?" The simulator takes the parts in schematic for the netlist. Breadboard are like wires, they build the netlist, but they are not part of the simulation. A part can have several spice lines and components (eg. a potentiometer has two resistors and a dc motor has a resistor and an inductor), so I grey out an item if it is not in the netlist, but discarting visual elements (rulers, logos, notes, labels, breadboards, etc.). However, I can change the code to use the family property instead of the class. This will fix the issue with the simulator, but in the long run would be nice to fix and obsolete this breadboard. Thus, all the breadboards will have the same behaviour.

For example, the breadboard class also modifies these functions:

bool Breadboard::stickyEnabled() {
    return false;
}
bool Breadboard::canFindConnectorsUnder() {
    return false;
}
ItemBase::PluralType Breadboard::isPlural() {
    return Plural;
}

Not sure how they affect the behaviour of the breadboard.

KjellMorgenstern commented 3 years ago

It just feels dirty that the half breadboard is not of class breadboard. So yes, I am fine with obsoleting the part, but I need a test, as it may break the circuits of hundreds of users (even if they don't have a 45 degree rotation) The part is to relevant for many circuits to take risks.

failiz commented 3 years ago

Of course! And it is not a high priority bug, no rush.

KjellMorgenstern commented 7 months ago

After fixing this, one will have to manually lookup the old half breadboard in case these effects are wanted: image