Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding BLDC_IFX007T Motor driver Shield from Infineon #330

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

pandayswarnam
Copy link

No description provided.

@mMerlin
Copy link
Contributor

mMerlin commented May 18, 2021

A part in the core repository should have a ".fzp" file in the core folder, NOT a ".fzpz" file. The .fzpz is for a self contained (standalone) part. It is a zip file that contains a .fzp plus .svg files that make up the part. I could not figure out how to view or download the .fzpz in the pull request, to see if it was just badly named, or if really is a binary file. A .fzp file contains xml in plain text.

@mMerlin
Copy link
Contributor

mMerlin commented May 18, 2021

There is convention for naming the svg files, to provide more information (in the name). See 4. File Naming Conventions

For shields, that is fairly simple. With "shield" in the description, package can be skipped. For views that do not include the description, package can be shield. For shields, all svg files could be part_description_view.svg. A bit more description is needed than the part number. The same applies to the (missing) fzp file. It needs to show that it is an arduino uno format motor shield.

@pandayswarnam
Copy link
Author

hi @mMerlin,
I have updated the files in correct format kindly have a look and give your comments for the same.
Replaced fzpz--> fzp file and updated other files as well.

@mMerlin
Copy link
Contributor

mMerlin commented Jul 9, 2021

I ran the part files through FritzingCheckPart. For core part files, that means creating a directory structure containing only the files to be included in the test.

./core/BLDC_Motor_Control_Shield_IFX007T_breadboard.fzp
./svg/core/breadboard/BLDC_Motor_Control_Shield_IFX007T_breadboard.svg
./svg/core/icon/BLDC_Motor_Control_Shield_IFX007T_breadboard_icon.svg
./svg/core/schematic/BLDC_Motor_Control_Shield_IFX007T_breadboard_schematic.svg
./svg/core/pcb/BLDC_Motor_Control_Shield_IFX007T_breadboard_pcb.svg

Here is a summary of what it reported, and what I noticed doing a manual inspection and smoke test in Fritzing. I had issues with my current environment, so I could not merge your part into core for testing. Instead, I used your files to create an fzpz part file, and used that. Note, this was a "SMOKE" test. There could be other problems that my quick check did not find.

The description information is excessive. A "SHORT" paragraph of information about the part is all that should be included. The advertising and detail information can be found using the url. The web page content should not be duplicated in the description.

The icon svg looks the same as the breadboard view. Instead of using a separate icon file, you can directly use the breadboard svg. Just make the image attribute for iconview the same as the image attribute for the breadboard view, then the icon svg can be removed.

SVG file names for core parts should (somewhat) follow the published 4. File Naming Conventions. The names should not include a guid type string. The moduleId can be (or include) a guid, but for core parts it is more common to match the fzp file name, possibly with a sequence or version number suffix added to the end. It just has to be a unique value across all parts, both in code, and any loaded fzpz part files.

(probably) related to the previous, the image file names specified for the view layers do not match the actual included files. That looks like an incomplete conversion from a standalone part file to the core parts. The svg file appear to have been renamed, but the meta data in the fzp was not updated.

The last several defined connectors do not have terminalIds specified for the schematic view. connector 209 through 215.

The connectors in the "gnd" bus do not match the connectors with a description and name of "GND". Connector numbers 179, 209, 210, 211 are marked as GND, but the "gnd" bus contains numbers 200, 213, 215. The "internal1" bus contains 2 (209 and 211) of the GND connectors.

connector200 specified in the gnd bus does not exist.

There are 2 connectors (182 and 212) marked as 5V VDD. They should be on a bus as well. Assuming they are connected together on the board.

6 of the connector description entries have a trailing space
-- IE <description>GND </description> instead of <description>GND</description>

Some of the connectors are marked as type="pad", others as type="male". Any connectors that should connect to a microcontroller (Arduino) board when the shield part is placed on top of it in breadboard view should be type="male". This is fine if the "pad" connectors are for pins that should not connect. If those are all board configuration jumpers, or screw terminal connections, they are correct.

The connectors are expected to be number starting from zero, up to (one less than) the number of connectors in the part. These are numbered in the range of 179 to 215 with a couple gaps. That should be from 0 to 25 for the connectors defined. Perhaps this was copied from a section of another part file that needed the larger range. Or the part copied from did not follow the rules either.

That label element in the fzp file is not a description. It is used as a prefix to number parts in the views when more than one exists in the sketch. Like "R" for resistors. A shield should be something like "Mod" (short for module). Perhaps something like "HAT", which is short, and used for raspberry pi. It should definitely be a single word (no spaces).

You could add "motor controller", or "motor driver" to the list of tags.

There are a few existing "motor driver" family parts in core. There is not much consistency, but perhaps adding <property name="package">shield</property> would help narrow down the selections going forward. Properties are shown in Inspector, and can be used to switch between parts in the same family, but that only works when the same property exists (with different values) in the different parts. So having all of the shields with that property would provide a quick method of separating from the discrete chips where the package is related to the footprint. And "shield", "hat", "board" (or module) could separate between Arduino, Raspberry Pi, and stand alone modules. Additional properties could be useful as well. I do not have list handy for what be most helpful for motor drivers. Perhaps start with current or power rating.

The schematicView terminalId values specified in the fzp file do not actually exist in the svg file. There are also connector and terminal references in the svg file that do not exist in the fzp. That needs to get lined up to work correctly. If it does not exist in the part, delete it from the svg (or change the name to something that does exist). Anything defined in the fzp (for a view) needs to exist in the svg for that view.

@KjellMorgenstern
Copy link
Member

There seems to be a mismatch in filename letter case. This is only an issue if the file system doesn't ignore letter cases -> it often goes unnoticed on windows.


Run python scripts/checkcase.py
Warning: missing ./core/BLDC_Motor_Control_Shield_IFX00[7](https://github.com/fritzing/fritzing-parts/runs/6374435614?check_suite_focus=true#step:6:7)T_breadboard.fzp breadboard/BLDC-SHIELD_IFX007T_63fdf44190600072626[8](https://github.com/fritzing/fritzing-parts/runs/6374435614?check_suite_focus=true#step:6:8)0d52d2235abe_5_breadboard.svg
Warning: missing ./core/BLDC_Motor_Control_Shield_IFX007T_breadboard.fzp schematic/BLDC-SHIELD_IFX007T_63fdf4419060007262680d52d2235abe_5_schematic.svg
Warning: missing ./core/BLDC_Motor_Control_Shield_IFX007T_breadboard.fzp pcb/BLDC-SHIELD_IFX007T_63fdf441[9](https://github.com/fritzing/fritzing-parts/runs/6374435614?check_suite_focus=true#step:6:9)060007262680d52d2235abe_5_pcb.svg
Warning: missing ./core/BLDC_Motor_Control_Shield_IFX007T_breadboard.fzp icon/BLDC-SHIELD_IFX007T_63fdf4419060007262680d52d2235abe_5_icon.svg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants