Skip to content

Adding support for wire length units #161

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.

Sign up for GitHub

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

Closed
zombielinux wants to merge 1 commit into from
Closed

Adding support for wire length units #161

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/wireviz/Harness.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def create_graph(self) -> Graph:
f'{cable.wirecount}x' if cable.show_wirecount else None,
f'{cable.gauge} {cable.gauge_unit}{awg_fmt}' if cable.gauge else None,
'+ S' if cable.shield else None,
f'{cable.length} m' if cable.length > 0 else None,
f'{cable.length} {cable.lengthunit}' if cable.length > 0 else None,
Comment on lines 171 to +173
Copy link
Collaborator

@kvid kvid Aug 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see an unfortunate difference in naming style between these two attribute pairs:

  • cable.gauge and cable.gauge_unit
  • cable.length and cable.lengthunit

Feel free to discuss naming conventions in a new issue, but I suggest we try to keep it consistent.

Copy link
Contributor

@stevegt stevegt Oct 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the underbar to length_unit in #171

cable.color, '<!-- colorbar -->' if cable.color else None],
'<!-- wire table -->',
[html_line_breaks(cable.notes)]]
Expand Down Expand Up @@ -365,7 +365,7 @@ def bom(self):
gauge_name = f' x {shared.gauge} {shared.gauge_unit}' if shared.gauge else ' wires'
shield_name = ' shielded' if shared.shield else ''
name = f'Cable{cable_type}, {shared.wirecount}{gauge_name}{shield_name}'
item = {'item': name, 'qty': round(total_length, 3), 'unit': 'm', 'designators': designators,
item = {'item': name, 'qty': round(total_length, 3), 'unit': shared.lengthunit, 'designators': designators,
Copy link
Collaborator

@kvid kvid Aug 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you risk making a BOM entry where total_length is the sum of several numeric values with different units. I can see at least two strategies to avoid this:

  1. Convert the values to a common unit before adding them together.
  2. Include the length unit in the cable_group to avoid joining entries with different length units.

I recommend discussing this choice (and if choice 1, what common unit to choose) in #7.

'manufacturer': remove_line_breaks(shared.manufacturer), 'mpn': remove_line_breaks(shared.mpn), 'pn': shared.pn}
bom_cables.append(item)
# bundles (ignores wirecount)
Expand Down