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

Fix group_connectors option not working #51

Merged
merged 2 commits into from
Apr 16, 2019

Conversation

Swij
Copy link
Contributor

@Swij Swij commented Apr 15, 2019

Closes #23

@SchrodingersGat
Copy link
Owner

SchrodingersGat commented Apr 15, 2019

Hi @Swij thanks for contributing. I like what you are going for here.

I would make two suggestions for now:

  1. Not all connectors are in Connector: library

Instead, something like if self.getLibName.lower().contains('connector') and other.getLibName...

  1. The library symbol names should match (even if they have been renamed in the schematic

example: Say you have a connector named "Motor" and another called "Battery" but they are both instances of Connector_Generic:Connector_1x02 then they can be matched.

counter example: If one connector is Connector_Generic:Connector_1x02 and the other is Connector_Generic:Connector_2x20 then they should not be considered equal.

In these cases the Connector_1x02 part would have been renamed to something like Motor but the libPart reference should still be accessible

@Swij
Copy link
Contributor Author

Swij commented Apr 16, 2019

Hey @SchrodingersGat, thanks for the quick feedback!

  1. Fair point! I've added a fix for that.
  2. Isn't that up to comparePartName() to decide? As long as "Part" is added to GROUP_FIELDS it should never group two connectors with different symbols.

@SchrodingersGat
Copy link
Owner

You're correct re: point 2!

Otherwise looks good. Thanks for contributing!

@SchrodingersGat SchrodingersGat merged commit fb5873e into SchrodingersGat:master Apr 16, 2019
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.

group_connectors option doesn't work
2 participants