[ID-707] First pass of BOM generation including Device Sheets for Altium projects.

[polypour]

Adds support for Device Sheets in Altium Projects when listing components or generating BOMs. Device Sheets are sheets external to the project that are references in it. The method for reading them involves either finding them in the given repo but outside the project (as in the case of a monorepo) or the user supplying a list of design reuse repositories which are scanned for the device sheet.

The components generated from device sheets usually have designators that are not very useful - the follow up PR #183 fixes that.

PR #183 was merged into this branch and this PR (#182) completes the Altium Device Sheet support for design reuse including the usage of board level annotations as applied by .Annotation files.

Outdated

• This PR works for Device Sheets located at the root level of a repo directory.
• When Device Sheets are nested in subdirectories, the _build_schdoc_hierarchy() function does not resolve the appropriate child path properly as determined from PrjPcb/crysknife ingestion. The above function needs to be modified to account for the SheetRef not pointing to the actual filename, which results in a KeyError.

Overall strategy for finding Device Sheets in an external repository:

• Search by exact filepath from the external repo as indicated by the repo name in the PrjPcb file, and the filepath from the root level of the repo as listed in the project file.
• If unable to find an exact match, try to find all matches by filename alone in all of the design reuse repos provided to the BOM generation function.
• Honor the option to return the first found match, i.e., honor a repo closest to index 0 in the list of design reuse repos.
• If duplicates (>= 2) are found for the Device Sheet in question, throw a ValueError indicating that duplicates were found.
• If unable to find the DeviceSheet, throw a ValueError indicating that no DeviceSheets were found.

[shrik450]

Can you add a test following the format in tests/test_utils.py? You will have to create a repo in the https://hub.allspice.io/AllSpiceTests organization as a fixture.

[shrik450]

Noting down some of the suggestions I made offline:

1. We clearly need different strategies for working with Device Sheets and Design Sheets, so it makes sense to create two different dictionaries for them instead of one of sheet names to JSONs. I'm thinking something like:

   @dataclass
   class ProjectDocument:
       path_from_project_root: str
   
       def __hash__(self):
           return hash(self.path_from_project_root)
   
   @dataclass
   class DeviceSheet:
       name: str # This is the name we'll find in `filename`
       repository: Repository
       path_from_repo_root: str # The actual path in the reuse repo
   
       def __hash__(self):
           return hash(self.name)
   
   # Later in list_components_for_altium
   project_documents: dict[ProjectDocument, dict] = {} # ProjectDocument to JSON
   device_sheets: dict[DeviceSheet, dict] = {} # DeviceSheet to JSON

   This assumes the restriction I discussed in the call, i.e. that a single project can only have one device sheet with a given name. This way, you can easily find which doc to use given a filename.

2. The above also implies that we loop through all the section of the project file and make a distinction between Document{} and DeviceSheet{} blocks, instead of the current system where we just check if the section has a path ending with SchDoc.

3. Re: The first match logic - I agree that it's useful to have something debuggable when you have multiple possible matches for a device sheet, but I don't think having this as a configuration option is useful. Having it error out when seeing multiple files will just annoy users into disabling the feature, and from then on we could end up silently picking the wrong file and they'd have no means of debugging it. I think the best thing to do here is to log (perhaps at the debug level) and then pick the first match. This way, users have the reasoning behind the choice and can tweak it by changing the order of design reuse repos, but they're not annoyed by the feature.

4. By caching the tree, I just mean having a dict in the list_components_for_altium function that maps design reuse repos to their trees. This dict can be passed down to functions that need it so we don't have to load the tree for every device sheet.

[notion-workspace]

[py-allspice] Support Device Sheets in list_components

[github-actions]

Coverage Summary

Total Project Coverage

• Line Coverage: 86.90% (2043/2351)
• Branch Coverage: 71.76% (432/602)

Coverage by File

File	Line Coverage	Branch Coverage	Lines (Covered/Total)	Branches (Covered/Total)
allspice/__init__.py	100.00%	100.00%	5/5	0/0
allspice/allspice.py	81.43%	68.42%	171/210	52/76
allspice/apiobject.py	83.58%	46.35%	1033/1236	89/192
allspice/baseapiobject.py	85.29%	75.00%	87/102	24/32
allspice/exceptions.py	100.00%	100.00%	14/14	0/0
allspice/ratelimiter.py	100.00%	100.00%	22/22	4/4
allspice/utils/__init__.py	100.00%	100.00%	0/0	0/0
allspice/utils/bom_generation.py	100.00%	100.00%	112/112	40/40
allspice/utils/core.py	94.12%	50.00%	16/17	1/2
allspice/utils/list_components.py	92.06%	86.89%	533/579	212/244
allspice/utils/netlist_generation.py	92.59%	83.33%	50/54	10/12

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• allspice/init.py (100%)
• allspice/utils/bom_generation.py (100%)
• allspice/utils/list_components.py (93.1%): Missing lines 237,244-245,982,990,1010,1023,1107-1108,1122,1189,1219-1220,1266,1361-1362,1365,1426-1427,1438,1441-1442,1453

Summary

• Total: 336 lines
• Missing: 23 lines
• Coverage: 93%

Diff Coverage Details

allspice/utils/list_components.py

Lines 233-241

  233     allspice_client.logger.info("Found %d SchDoc files", len(project_documents))
  234     allspice_client.logger.info("Found %d Device Sheet files", len(device_sheets))
  235 
  236     if not project_documents:
! 237         raise ValueError("No Project Documents found in the PrjPcb file.")
  238 
  239     try:
  240         annotations_data = _fetch_and_parse_annotation_file(repository, prjpcb_file, ref)
  241         allspice_client.logger.info("Found annotations file, %d entries", len(annotations_data))

---

Lines 240-249

  240         annotations_data = _fetch_and_parse_annotation_file(repository, prjpcb_file, ref)
  241         allspice_client.logger.info("Found annotations file, %d entries", len(annotations_data))
  242     except Exception as e:
  243         if device_sheets:
! 244             allspice_client.logger.warning("Failed to fetch annotations file: %s", e)
! 245             allspice_client.logger.warning("Component designators may not be correct.")
  246         annotations_data = {}
  247 
  248     # Mapping of schdoc file paths from the project file to their JSON
  249     schdoc_jsons: dict[str, dict] = {}

---

Lines 978-986

  978                 designator = variation_details["Designator"]
  979                 unique_id = variation_details["UniqueId"]
  980                 kind = variation_details["Kind"]
  981             except KeyError:
! 982                 logger.warning(
  983                     "Designator, UniqueId, or Kind not found in details of variation "
  984                     f"{variation_details}; skipping this variation."
  985                 )
  986                 continue

---

Lines 986-994

  986                 continue
  987             try:
  988                 kind = VariationKind(int(variation_details["Kind"]))
  989             except ValueError:
! 990                 logger.warning(
  991                     f"Kind {variation_details['Kind']} of variation {variation_details} must be "
  992                     "either 0, 1 or 2; skipping this variation."
  993                 )
  994                 continue

---

Lines 1006-1014

  1006             except KeyError:
  1007                 # This can happen sometimes - Altium allows param variations
  1008                 # even when the component is not fitted, so we just log and
  1009                 # ignore.
! 1010                 logger.warning(
  1011                     f"ParamVariation{variation_id} found for component {designator} either before "
  1012                     "the corresponding Variation or for a component that is not fitted.\n"
  1013                     "Ignoring this ParamVariation."
  1014                 )

---

Lines 1019-1027

  1019                     variation_details["ParameterName"],
  1020                     variation_details["VariantValue"],
  1021                 )
  1022             except KeyError:
! 1023                 logger.warning(
  1024                     f"ParameterName or VariantValue not found in ParamVariation{variation_id} "
  1025                     "details."
  1026                 )
  1027                 continue

---

Lines 1103-1112

  1103     ).casefold()
  1104     project_repo_tree = _fetch_all_files_in_repo(project_repository)
  1105     for filepath in project_repo_tree:
  1106         if device_sheet_path_from_repo_root == filepath.casefold():
! 1107             logger.info("Found device sheet %s in project repository; using that.", device_sheet)
! 1108             return (
  1109                 project_repository,
  1110                 pathlib.PurePosixPath(filepath),
  1111             )

---

Lines 1118-1126

  1118             if device_sheet_name == file_path.name.casefold():
  1119                 matches.append((repo, file_path))
  1120 
  1121     if len(matches) == 0:
! 1122         raise ValueError(
  1123             f"No matching device sheet found for {device_sheet_path} in the design reuse "
  1124             "repositories.",
  1125         )

---

Lines 1185-1193

  1185         raise Exception(
  1186             f"Could not find annotation file {annotation_file_path} in repository {repository.url}."
  1187         )
  1188     if len(matches) > 1:
! 1189         raise Exception(
  1190             f"Multiple matches found for annotation file {annotation_file_path} in repository "
  1191             f"{repository.url} when checking case-insensitively: {matches}."
  1192         )

---

Lines 1215-1224

  1215         logical_designator = designator_manager_section[f"LogicalDesignator{change_number}"]
  1216         physical_designator = designator_manager_section[f"PhysicalDesignator{change_number}"]
  1217 
  1218         if unique_id in changes:
! 1219             repository.allspice_client.logger.warning("Multiple changes found for %s", unique_id)
! 1220             continue
  1221 
  1222         changes[unique_id] = {"from": logical_designator, "to": physical_designator}
  1223         change_number += 1

---

Lines 1262-1270

  1262         annotation_for_component = None
  1263         for id_to_test in ids_to_test:
  1264             annotation_for_component = annotations_by_sheet.get(id_to_test)
  1265             if annotation_for_component:
! 1266                 break
  1267 
  1268         if not annotation_for_component:
  1269             logger.debug(
  1270                 "No annotation found for component %s by unique ID, trying by designator.",

---

Lines 1357-1369

  1357     """
  1358 
  1359     try:
  1360         annotate_section = prjpcb_ini["Annotate"]
! 1361     except KeyError:
! 1362         logger.warning(
  1363             "Annotate section not found in PrjPcb; Designators of componets within Device Sheets may be incorrect."
  1364         )
! 1365         return hierarchy
  1366 
  1367     paths_to_documents: dict[str, list[str]] = {}
  1368     document_index = 0

---

Lines 1422-1431

  1422         for child in children:
  1423             if child.kind == AltiumChildSheet.ChildSheetKind.DOCUMENT:
  1424                 # We don't need to change the IDs of documents - they're
  1425                 # already correct.
! 1426                 final_children.append(child)
! 1427                 continue
  1428 
  1429             children_by_name.setdefault(child.name, []).append(child)
  1430 
  1431         for child_name, children_of_name in children_by_name.items():

---

Lines 1434-1446

  1434             child_name += ".SchDoc"
  1435             paths_to_children = paths_to_documents.get(child_name, [])
  1436 
  1437             if not paths_to_children:
! 1438                 logger.warning(
  1439                     f"Could not find any paths for {child_name} in the PrjPcb file; skipping."
  1440                 )
! 1441                 final_children.extend(children_of_name)
! 1442                 continue
  1443 
  1444             ids: set[str] = set()
  1445             for path in paths_to_children:
  1446                 if not path.startswith(path_to_this_sheet):

---

Lines 1449-1457

  1449                 path = path.removeprefix(path_to_this_sheet + "\\")
  1450 
  1451                 if "\\" in path:
  1452                     # This path is deeper in the hierarchy, we can't use it.
! 1453                     continue
  1454 
  1455                 # Now the path is the UniqueID of the child sheet
  1456                 ids.add(path)

---

[kdumontnu]

one thing we might need to follow up on is that there are multiple "D1"s (for each of the sub-references).
Altium splits these definitions into "PhysicalDesignator" and "LogicalDesignator" (though it's not clear how this schema works to me).
This is probably fine for now, but we should add this to the list of things to come back to. Users might have issues.
Altium has an "Annotate Compiled Sheets" tool. I couldn't get it to work. @polypour is this what you were working with? Did you generate a BOM we can compare to?

[shrik450]

I think this is part of what @polypour was referring to when he mentioned that he wasn't able to re-number all the references. I can re-generate the BOM if/when we figure that out and see what we get!

[kdumontnu]

Yeah, agreed. Unfortunately, I suspect that it's not going to be as simple as updating the reference designators in the Altium test design and having that produce the same BOM output as Altium.

Since the device sheets are shared between multiple "parent" projects, the designators in the sub-sheets can't be shared in all of those parent instances (there will always be the potential for collisions). EITHER, the designators are mapped from the sub-sheet to the parent sheet (which is what I think the .Annotation file does) OR there is some suffix appended.

That seems to be indicated here: https://www.altium.com/documentation/altium-designer/device-sheets#annotating-compiled-device-sheets#annotating-compiled-device-sheets

As discussed, we need to continue with what we have to get some feedback on the initial implementation, but I'll create a ticket to come back to this.

Also, I suspect my inability to generate the annotation output is either a bug in Altium or because I'm running in parallels. I'm seeing other oddities too (like not being able to create and update a PCB). @polypour are you running on native Windows?

[shrik450]

Also, tagging @McRaeAlex as a reviewer since I've also contributed to this PR.

[shrik450]

Missed this in my commit but design_reuse_repos should go at the end of the args list to avoid a breaking change - I'll fix it.

[kdumontnu]

Some questions around case-sensitivity & variable names, but otherwise looks great to me.

Next steps:

• Can device sheets reference other device sheets? I expect so. Perhaps these needs to become recursive.
• We should update to allow reuse_repos to be defined in assemblies.yaml
• We should follow-up on designator generation using an .annotations file
• Make similar capability work for DSN & sdax

Anything else?

[kdumontnu]

We say document here, but I since we are expecting these all to be schematics we should change schematic_jsons

[shrik450]

Device Sheets are also technically schematics, which is why the distinction is between Document as in the project and DeviceSheet as in the project, but I can rename these to be schematic_documents to make that clearer.

[shrik450]

Re: the questions:

• Can device sheets reference other device sheets? I expect so. Perhaps these needs to become recursive.

Yes, and this is handled - in the hierarchy building function we go through refs in the device sheets as well.

• We should update to allow reuse_repos to be defined in assemblies.yaml

I brought this up when we were discussing the proposal too, and we need to think about how this is defined in generate-bom which also needs an update to handle this. I'll make that change when this is through.

• Make similar capability work for DSN & sdax

This is non-trivial because we currently do not parse or export sheet references in DSN, so it will also take backend effort.

[McRaeAlex]

Please update the PR description to be up to date, it seems some configuration stuff has been removed / changed.

[McRaeAlex]

I am unfamiliar with device sheets, is there every a situation where device_sheet_name may overwrite a schdoc_file in sheets_to_components?

[shrik450]

There shouldn't be, because the schdoc files are paths while device sheets are names. Theoretically it is possible but in the same situation Altium would also have difficulty identifying which sheet it is using. Might be worth testing @polypour.

[shrik450]

(Note: we tested this out in Altium and Altium also chokes in this case - it thinks the project document is the device sheet of the same name. Given that this is unexpected behaviour, we're not supporting it.)

[polypour]

@shrik450 comment above is true. Altium identifies device sheets by name, primarily. I've tried including multiple folders with the same device sheets under different directory nestings with the same names and it only picks up the first copy seen.

[shrik450]

Self-approved to clear my request for changes - please ignore that!

[jackHay22]

👍

[kdumontnu]

Let's wait for @polypour to merge this. Last I saw they were still doing some validation.

Actually, I might need to merge it since it looks like @shrik450 was right and we'll have to force merge because of the pending review. LMK when it's all set and I'll make it happen.

[shrik450]

I've bumped the version and changelog in this repo so we can tag a release as soon as we merge this 👍

Reviewed by Shrikanth, Jack, Kyle, Gautam. Tested with .Annotations file support with repetitions and hierarchies, cross checking against UniqueIds.

[polypour]

Stress tested and validated locally and merged. Thank you @shrik450 and @kdumontnu. 🙏