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

add ultrasonic distance sensor "us-100" #328

Merged
merged 6 commits into from
Jun 1, 2021

Conversation

moritzmhmk
Copy link
Contributor

No description provided.

@mMerlin
Copy link
Contributor

mMerlin commented May 7, 2021

The label element in the fzp file is used by Fritzing as a prefix for generated labels. A number is added to that for each copy of the part added to a sketch. A label ending with a number does not work well. I see 2 other core parts with a family of "distance sensor". One has a label of "J", the other of "HC-SR04 Sensor". Nether or those are particularly "good" labels. Labels should be kept short, but somewhat unique (by part type or category). Typically a very few characters. This is used to (sort of) group related parts together. Resistors have a label of "R". All resisters, regardless of value, power rating, accuracy will be given a label of "R" plus a sequence number. Transistors typically have a label of "Q", basic chips "U", switches "S", I would suggest "SEN" for sensors, to keep it short, and something that should be recognizable. You have the variant property set to "US-100". If that text is desired as part of the part view labelling, the variant field can be enabled, by the user, for the specific view and part. The label is not a good place for the information.

The url element has been left blank. A link valid link would be a good idea.

From the dimmesions, you have 0.2 inch long connection lines, with 0.2 inch spacing between them. 0.1 inch is the current convention, although a lot of existing part do not follow that.

The fzp file includes terminalId attributes for the schematicView, pointing to connector«n»terminal ids. Those do not exist in the schematic svg. That will cause wires to connect to the middle of the connector lines, instead of the end (where the "…terminal" elements should be).

This is from looking at the committed files, without loading as a part in Fritzing.

@moritzmhmk
Copy link
Contributor Author

Thanks for the review. I changed the label and connector distance, fixed the terminals and added an url to the Adafruit product page.

@mMerlin
Copy link
Contributor

mMerlin commented May 8, 2021

I have never seen a terminalId use a path element before. Usually line or rect, sometimes circle. Path should work fine. However, the terminalId elements are used for Fritzing processing. Except for odd cases, they should have stroke="none". With the current size, position, color, they are effectively not seen, but setting stroke to none really makes them invisible. Like the previous bare connector, it is the center of the terminal element extent that Fritzing uses for the wire connection point. For complete accuracy, make the paths 2 long, and change the start point back by 1, so the mid point is where you want the wire to connect to.

It looks like there is extra white space both above and below the schematic graphics. The svg shows a height of 0.6 inches. You changed the viewbox from 600 to 500, but did not change the height. 0.5 isn't correct either. The graphics is only 0.3 (actually 0.31, when the stroke width is accounted for) tall. Inkscape "Resize Page to Selection" can be used to fix that, but it also messes up some other things that are currently clean. The "actual" graphic content extent is 0.71in by 0.31in, which should have viewbox of 0 0 710 310, but then all of the element coordinates would need to be offset by +5 x and -95 y. That handles the stroke-width of 10, half of which is rendered on each side of the element coordinates specified.

That offset does NOT break alignment with the 0.1in grid in Fritzing. It is the relative distances between the connectors and terminals that matter for grid alignment.

Have you tried running your part through FritzingCheckPart? It is a bit tricky to set up to process single (or small subset of) core parts. The folder structure needs to match, with only the files to be validated in them. I have used a script to pull my new or modified part files from the development folder (which has all core parts) into an otherwise empty folder structure, and validating there. That would have reported at least the missing terminal elements.

As "4. File Naming Conventions" in the graphic standards document says, any valid (for operating system and file system) file names will technically work. To make the files easier to find and reuse (for other parts), a convention exists. The names should include information about what file contains. Specifically (where it applies), «fairly short» part description, number of pins, package, spacing, view name. "us" in the filename may obviously (to some) be ultrasonic, but a bit longer would often help. The existing HC-SR04 sensors is not a good example for this. It uses "hc-sr04_bf8299a_002" for the part file name itself, and the svg file in every view. "HC-SR04" is a relatively well know part number, so make a reasonable description. "us-100" is less descriptive, and I think both should include something that indicates ultrasonic sensor.

We want new parts, but we also want to get a lot of cleanup done for the existing parts. Would prefer not to create more cleanup work with the new parts.

@moritzmhmk
Copy link
Contributor Author

Thank you again for your extensive feedback.

I did initially create all files in this repository which despite the naming is not a fork of fritzing-parts but instead a symlink to my ~/Fritzing/parts folder which I found useful for testing. All changes mentioned are committed there individually.

I do code my vector graphics "manually" i.e. I have the code opened in the Atom editor with a live preview of the resulting image. If this might be helpful to others I could rebuild the template graphics to also have a minimal structure and use meaningful scaling (i.e. inch for size and mil for viewport).

I misinterpreted the schematic template - the red rectangle seemed to imply a 0.1" border, but I now realized that this due to the upper pin being present. I fixed the sizing.
schematic template

I must have missed the FritzingCheckPart tool when reading about part creation.

I did fix all errors except for Found a drawing element before a layerId (or no layerId) which persists since in the breadboard svg I define the ultrasonic tube in def and use it later.

In schematic view the "hybrid" connector 4 (GND-2) is available as the center of the entire part. Do you have any clue why this happens (it does also happen with the current version of this PR - I just didn't realize it before)? I have attached a screenshot to illustrate the problem.

Screenshot

My first thought when seeing the FritzingCheckPart repository was that the README needs to be fixed. I found there to already be a pull request by @KjellMorgenstern. Please consider merging this into the fritzing fork. Furthermore, I would suggest a separation of concerns regarding formatting and logical checking of files (following the "unix philosophy" of doing one thing well). There are many tools for formatting available already. I did use the prettier plugin for atom (with the html settings - see .prettierrc) to do the code formatting. Since prettier is an opinionated code formatter it might not fit this project I would then still suggest using a (more configurable) formatter and to publish a config file for that instead of using a custom formatter as is currently the case. (Maybe I should create a separate issue for this?)

@mMerlin
Copy link
Contributor

mMerlin commented May 8, 2021

I do code my vector graphics "manually" i.e. I have the code opened in the Atom editor with a live preview of the resulting image

I do much the same. I used atom a couple years ago, but currently use Visual Studio Code. I recently found a vscode extension for SVG that provides an in editor preview window that updates (with a little lag) as you type. No need to save before seeing changes.

Getting the template files updated is on the list … along with a lot of other things.

I must have missed the FritzingCheckPart tool when reading about part creation.

FritzingCheckPart is a relatively recent addition. Written by Peter vanepp (in the forums). Kjell forked his repository into fritzing. It has never been added into the official process, and I do not think it is in the main documentation any place. Information about it is in the forums. I think Kjell has added it to the workflows for PRs. That is (I think) part of one of python-scripts that currently show as pending. There are too many of the existing core parts that fail the checks to be used fully automatically. Which is why I said it was a little tricky to use with core parts. It is easier to use with fzpz part files.

I did fix all errors except for Found a drawing element before a layerId (or no layerId) which persists since in the breadboard svg I define the ultrasonic tube in def and use it later.

I am not sure that is safe. Fritzing was written to use SVG Tiny 1.2, plus a few extensions. I do not believe that def is included in that. The limit is partly what QT supports for rendering, but not only that. With sketches containing multiple parts, the svg for each gets 'merged'. That code does not handle things like def well, probably because it is not unique (after merging). That would need a lot smarter code. From experience, only the actual graphic elements (inside of the "breadboard" layer (group)) are merged. As a test, create a sketch with two copies of your part, and export as image, SVG. Check the result closely. The same applies for gerber export, but so far I have found that if SVG export works, so does gerber. At least for things related to the svg content.

The rules for using hybrid connectors are not well documented. It may not be quite correct, but I have gotten it to work by including the hybrid connector element in the svg file, but making it invisible (no fill, no stroke). Since that is part of a bus group, as an extra safety measure, I put it at the same coordinates as the other element for the bus.

Furthermore, I would suggest a separation of concerns regarding formatting and logical checking of files (following the "unix philosophy" of doing one thing well).

You could open an issue in one of the FritzingCheckParts repositories. I am not satisfied with the formatting it does either. I believe that it is completely home grown code, and some of the global choices it makes are not the best for specific cases.

Personally, I use it like 'lint'. I run it against a temporary copy of my files, then use the output messages to incorporate fixes in the originals. It does not compete well with properly structured hand coded svg. It does an excellent job of cleaning up some of the mess left by general svg editors like Inkscape.

Looking at the latest commit diffs, I am not sure that the adjusted schematic viewbox will work. Memory says that Fritzing wants the viewbox to always start with 0 0. The actual coordinates may need adjusting to match. I HAVE gotten around that by adding a wrapper group around the whole content with a translate transform. That is frowned on by many of the more advanced users creating parts. There are some cases where Fritzing does not handle transforms properly. The conditions that trigger the problem are not obvious. I have only had a single case where I needed to unwrap the transform to get the part to work correctly. I think the problem is mostly matrix transforms. Which are created a lot when using Inkscape. The solution there is to have Inkscape 'ungroup' (multiple times), forcing it to incorporate the transforms into the inner element coordinates. Hand coded svg uses fewer, more well defined/structured transforms, and appears to trigger the problems less often.

Also, the width should be 0.71in, to account for the stroke width going past the end point of the connector lines, and match the extent of the viewbox.

@moritzmhmk
Copy link
Contributor Author

I do much the same. I used atom a couple years ago, but currently use Visual Studio Code. I recently found a vscode extension for SVG that provides an in editor preview window that updates (with a little lag) as you type. No need to save before seeing changes.

The same functionality is available as a Atom plugin. I did try VS Code but am too used to my heavily customized Atom setup to switch.

Getting the template files updated is on the list … along with a lot of other things.

I created a little web app for creating simple graphics for the schematic view here.

I did fix all errors except for Found a drawing element before a layerId (or no layerId) which persists since in the breadboard svg I define the ultrasonic tube in def and use it later.

I am not sure that is safe. Fritzing was written to use SVG Tiny 1.2, plus a few extensions. I do not believe that def is included in that. The limit is partly what QT supports for rendering, but not only that. With sketches containing multiple parts, the svg for each gets 'merged'. That code does not handle things like def well, probably because it is not unique (after merging). That would need a lot smarter code. From experience, only the actual graphic elements (inside of the "breadboard" layer (group)) are merged. As a test, create a sketch with two copies of your part, and export as image, SVG. Check the result closely. The same applies for gerber export, but so far I have found that if SVG export works, so does gerber. At least for things related to the svg content.

SVG Tiny supports def but Fritzing does indeed not copy the content of def on export. I moved everything into a (hidden) subgroup of breadboard. Now also the gradients work on export (which is a bug I noticed for other parts before).

The rules for using hybrid connectors are not well documented. It may not be quite correct, but I have gotten it to work by including the hybrid connector element in the svg file, but making it invisible (no fill, no stroke). Since that is part of a bus group, as an extra safety measure, I put it at the same coordinates as the other element for the bus.

When the size is zero the entire part becomes selectable as the hybrid pin. When the pin has a size other than zero it can be hovered at its (invisible) position. Placing it outside the visible area solves the problem (I also checked hovering the exact position).

You could open an issue in one of the FritzingCheckParts repositories. I am not satisfied with the formatting it does either. I believe that it is completely home grown code, and some of the global choices it makes are not the best for specific cases.

Personally, I use it like 'lint'. I run it against a temporary copy of my files, then use the output messages to incorporate fixes in the originals. It does not compete well with properly structured hand coded svg. It does an excellent job of cleaning up some of the mess left by general svg editors like Inkscape.

That is exactly what I did. Btw. you can set debug to "1" to disable the files being altered.

Looking at the latest commit diffs, I am not sure that the adjusted schematic viewbox will work. Memory says that Fritzing wants the viewbox to always start with 0 0. The actual coordinates may need adjusting to match. I HAVE gotten around that by adding a wrapper group around the whole content with a translate transform. That is frowned on by many of the more advanced users creating parts. There are some cases where Fritzing does not handle transforms properly. The conditions that trigger the problem are not obvious. I have only had a single case where I needed to unwrap the transform to get the part to work correctly. I think the problem is mostly matrix transforms. Which are created a lot when using Inkscape. The solution there is to have Inkscape 'ungroup' (multiple times), forcing it to incorporate the transforms into the inner element coordinates. Hand coded svg uses fewer, more well defined/structured transforms, and appears to trigger the problems less often.

Also, the width should be 0.71in, to account for the stroke width going past the end point of the connector lines, and match the extent of the viewbox.

The viewport position is indeed ignored when exporting as svg. I did move it to a transform for all views.

@moritzmhmk
Copy link
Contributor Author

Are any further changes required before merging?

@KjellMorgenstern
Copy link
Member

Looks good! The generator output seems great. Note: For larger schematics, Vcc should be on top, while GND should be at the bottom. Also, grouping of connectors could be done by using submodules. I am working on improving the fritzing mystery part to achieve this.
I have run scour on the svg files, the formatting looks more readable, and the files size is reduced to around 70% .
But I think the existing PR is already tested, so lets integrate it.

@KjellMorgenstern KjellMorgenstern merged commit 6f04697 into fritzing:develop Jun 1, 2021
@moritzmhmk moritzmhmk deleted the us-100 branch June 5, 2021 10:50
@moritzmhmk
Copy link
Contributor Author

I happened to rewrite the schematic generator in Elm as a learning project. Maybe this will be useful to someone ;)

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