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 variant logic for '+'-prefixed board variants #136

Closed
wants to merge 4 commits into from
Closed
Changes from 3 commits
Commits
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
35 changes: 21 additions & 14 deletions kibom/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,21 +328,28 @@ def isFitted(self):
if opt.lower() in DNF:
return False

# Variants logic
opts = check.split(",")
for opt in opts:
opt = opt.strip()
# Any option containing a DNF is not fitted
if opt in DNF:
return False
Comment on lines -336 to -337
Copy link
Contributor

Choose a reason for hiding this comment

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

What about this check?

Copy link
Author

Choose a reason for hiding this comment

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

Another good catch! I got confused by check above which does almost the same, but that's for space-separated options...

# Options that start with '-' are explicitly removed from certain configurations
if opt.startswith("-") and str(opt[1:]) in self.prefs.pcbConfig:
return False
# Options that start with '+' are fitted only for certain configurations
if opt.startswith("+") and opt[1:] not in self.prefs.pcbConfig:
# Variants logic (comma-separated list)
opts = [s.strip() for s in check.split(",")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a loop here? The current code uses another (recycled) loop for this.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't want to duplicate calls to strip() in both loops, so factored it out.

included_configs = set(opt[1:] for opt in opts if opt.startswith("+"))
excluded_configs = set(opt[1:] for opt in opts if opt.startswith("-"))

# Logic here:
# - If pcbConfig specifies variant from excluded_configs, component
# is excluded.
# - Otherwise, if included_configs is empty, component is included.
# - Otherwise, component is included only if pcbConfig specifies
# variant which is a member of included_configs.

included = True
if included_configs:
included = False
for config in self.prefs.pcbConfig:
if config in excluded_configs:
return False

return True
if config in included_configs:
included = True

return included

def isFixed(self):
""" Determine if a component is FIXED or not.
Expand Down