|
1 | 1 | --- |
2 | | -name: fix-drawing-sync-metadata |
3 | | -overview: Write config description and tab number to SW file immediately on edit commit (blur/Enter), so sync-metadata always reads fresh data. |
| 2 | +name: Fix Drawing Sync Metadata |
| 3 | +overview: "Fix two bugs: (1) Response mismatch causing drawing sync to receive empty parent properties, and (2) SW API loading assembly components when it should use DM API instead." |
4 | 4 | todos: |
5 | | - - id: write-config-desc-immediately |
6 | | - content: Modify handleConfigDescriptionChange to write description to SW file immediately via setProperties |
| 5 | + - id: fix-api-selection |
| 6 | + content: Change GetPropertiesFast to only use SW API when the specific file is open in SW, otherwise use DM API |
7 | 7 | status: pending |
8 | | - - id: write-config-tab-immediately |
9 | | - content: Modify handleConfigTabChange to write tab number to SW file immediately via setProperties |
| 8 | + - id: fix-response-id |
| 9 | + content: Ensure all C# CommandResult responses include the requestId for proper matching |
10 | 10 | status: pending |
11 | | - - id: test-sync-metadata |
12 | | - content: Test that editing config description/tab writes to SW file immediately, and sync-metadata on drawing reads the updated values |
| 11 | + - id: verify-fifo-fallback |
| 12 | + content: Review FIFO fallback logic in solidworks.ts to understand when mismatches occur |
13 | 13 | status: pending |
14 | 14 | --- |
15 | 15 |
|
16 | | -# Fix Config Description Not Syncing to Drawing |
| 16 | +# Fix Drawing Sync Metadata Bugs |
17 | 17 |
|
18 | | -## Problem Analysis |
| 18 | +## Problem 1: Drawing Sync Gets Empty Parent Properties |
19 | 19 |
|
20 | | -When a user edits a description on a configuration row of a part/assembly in BluePLM, the edit is stored only in memory (`pendingMetadata.config_descriptions`). When the user then runs "Sync Metadata" on a drawing that references that part's configuration, the command reads from the **disk file** (via `getProperties`) which has the OLD description. |
| 20 | +The log shows that when syncing metadata on a drawing, the `getProperties` call for the parent model returns empty data even though the SW service successfully reads the properties. |
21 | 21 |
|
22 | | -**Current Flow** (broken): |
| 22 | +**Evidence from log:** |
23 | 23 |
|
24 | | -1. User edits description on config row of `stator.SLDPRT` - stored ONLY in memory |
25 | | -2. User right-clicks drawing `stator.SLDDRW`, clicks "Sync Metadata" |
26 | | -3. `pullDrawingMetadata()` reads parent model properties from disk via `getProperties()` |
27 | | -4. Disk still has old description - sync-metadata pulls stale data to drawing |
| 24 | +- Line 980: `getProperties (id: 48) completed in 2ms` - impossibly fast |
| 25 | +- Lines 997-1046: SW Service logs reading `BR-107166` from file |
| 26 | +- Line 1063: Electron receives `filePropertyCount: 0` |
28 | 27 |
|
29 | | -**Why "Generate BR Number" works**: It calls `saveConfigsToSWFile()` which writes ALL pending metadata (including `config_descriptions`) to the SW file, so sync-metadata reads the updated value. |
| 28 | +**Root Cause:** Response mismatch in the IPC layer. Looking at [solidworks.ts](electron/handlers/solidworks.ts) lines 836-856: |
30 | 29 |
|
31 | | -## Solution |
| 30 | +- If `requestId` in response doesn't match pending requests, it falls back to FIFO matching |
| 31 | +- A ping response without requestId gets matched to the wrong request |
32 | 32 |
|
33 | | -**Write config description to SW file immediately** when the user commits the edit (blur or Enter). This keeps the SW file as the single source of truth and ensures sync-metadata always reads fresh data. |
| 33 | +**Fix:** Ensure all C# service responses include `requestId` for proper matching. |
34 | 34 |
|
35 | | -## Implementation |
| 35 | +## Problem 2: Component Files (WireAssembly.SLDPRT) Opening in SolidWorks |
36 | 36 |
|
37 | | -### File: [src/features/source/browser/hooks/useConfigHandlers.ts](src/features/source/browser/hooks/useConfigHandlers.ts) |
38 | | - |
39 | | -#### 1. Modify `handleConfigDescriptionChange` (lines 273-292) |
40 | | - |
41 | | -Add immediate write to SW file via `setProperties`: |
42 | | - |
43 | | -```typescript |
44 | | -const handleConfigDescriptionChange = useCallback(async (filePath: string, configName: string, value: string) => { |
45 | | - const { files, fileConfigurations } = usePDMStore.getState() |
46 | | - |
47 | | - const file = files.find(f => f.path === filePath) |
48 | | - if (!file) return |
49 | | - |
50 | | - // Update config in store (for immediate UI feedback) |
51 | | - const configs = fileConfigurations.get(filePath) |
52 | | - if (configs) { |
53 | | - const updated = configs.map(c => c.name === configName ? { ...c, description: value } : c) |
54 | | - usePDMStore.getState().setFileConfigurations(filePath, updated) |
55 | | - } |
56 | | - |
57 | | - // Update pending metadata (for persistence across app restart) |
58 | | - const existingDescs = file.pendingMetadata?.config_descriptions || {} |
59 | | - usePDMStore.getState().updatePendingMetadata(filePath, { |
60 | | - config_descriptions: { ...existingDescs, [configName]: value } |
61 | | - }) |
62 | | - |
63 | | - // Write to SW file immediately |
64 | | - try { |
65 | | - const result = await window.electronAPI?.solidworks?.setProperties(filePath, { 'Description': value }, configName) |
66 | | - if (result?.success) { |
67 | | - addToast('success', `Saved description to ${configName}`) |
68 | | - } else { |
69 | | - addToast('error', `Failed to save description: ${result?.error || 'Unknown error'}`) |
70 | | - } |
71 | | - } catch (err) { |
72 | | - log.error('[ConfigHandlers]', 'Failed to write config description to SW file', { filePath, configName, error: err }) |
73 | | - addToast('error', 'Failed to save description to file') |
74 | | - } |
75 | | -}, [addToast]) |
| 37 | +**Confirmed:** WireAssembly.SLDPRT is a component inside BAR-XT-ASM.SLDASM. It was NOT open before sync metadata. |
| 38 | + |
| 39 | +**Timeline from log:** |
| 40 | + |
| 41 | +- Line 715 (22:18:34.260): `No documents open in SolidWorks` |
| 42 | +- Line 883 (22:18:37.687): `Open doc: WireAssembly.SLDPRT` - appeared during sync! |
| 43 | + |
| 44 | +**Root Cause:** In [Program.cs](solidworks-service/BluePLM.SolidWorksService/Program.cs) lines 426-429: |
| 45 | + |
| 46 | +```csharp |
| 47 | +// If SolidWorks is RUNNING (even with other files open), use SW API for consistency |
| 48 | +if (_swApi != null && _swApi.IsSolidWorksRunning()) |
| 49 | +{ |
| 50 | + return _swApi.GetCustomProperties(filePath, ...); |
| 51 | +} |
76 | 52 | ``` |
77 | 53 |
|
78 | | -#### 2. Modify `handleConfigTabChange` (lines 247-266) |
79 | | - |
80 | | -Add immediate write to SW file via `setProperties`. Also need to recalculate the full `Number` property (base + tab): |
81 | | - |
82 | | -```typescript |
83 | | -const handleConfigTabChange = useCallback(async (filePath: string, configName: string, value: string) => { |
84 | | - const { files, fileConfigurations } = usePDMStore.getState() |
85 | | - |
86 | | - const file = files.find(f => f.path === filePath) |
87 | | - if (!file) return |
88 | | - |
89 | | - const upperValue = value.toUpperCase() |
90 | | - |
91 | | - // Update config in store |
92 | | - const configs = fileConfigurations.get(filePath) |
93 | | - if (configs) { |
94 | | - const updated = configs.map(c => c.name === configName ? { ...c, tabNumber: upperValue } : c) |
95 | | - usePDMStore.getState().setFileConfigurations(filePath, updated) |
96 | | - } |
97 | | - |
98 | | - // Update pending metadata |
99 | | - const existingTabs = file.pendingMetadata?.config_tabs || {} |
100 | | - usePDMStore.getState().updatePendingMetadata(filePath, { |
101 | | - config_tabs: { ...existingTabs, [configName]: upperValue } |
102 | | - }) |
103 | | - |
104 | | - // Write to SW file immediately |
105 | | - try { |
106 | | - const baseNumber = file.pendingMetadata?.part_number ?? file.pdmData?.part_number ?? '' |
107 | | - const props: Record<string, string> = { 'Tab Number': upperValue } |
108 | | - |
109 | | - // Also update the full Number property (base + tab) |
110 | | - if (baseNumber && upperValue) { |
111 | | - props['Number'] = `${baseNumber}-${upperValue}` // TODO: use serialization settings for separator |
| 54 | +This is **overly aggressive** - it uses the full SW API whenever SolidWorks is running, even if the target file isn't open. When SW API opens an assembly via `OpenDoc6`: |
| 55 | + |
| 56 | +1. SolidWorks loads ALL component references (like WireAssembly.SLDPRT) |
| 57 | +2. `CloseDoc` only closes the main assembly |
| 58 | +3. **Component files remain orphaned** in SolidWorks session |
| 59 | + |
| 60 | +The **Document Manager API** can read properties without loading files into SolidWorks at all! |
| 61 | + |
| 62 | +## Implementation |
| 63 | + |
| 64 | +### Step 1: Fix API Selection Logic (PRIMARY FIX) |
| 65 | + |
| 66 | +In [Program.cs](solidworks-service/BluePLM.SolidWorksService/Program.cs) `GetPropertiesFast`, change the logic to: |
| 67 | + |
| 68 | +- Only use SW API if the **specific file** is already open in SolidWorks |
| 69 | +- Otherwise use DM API (fast, doesn't load files into SW) |
| 70 | +```csharp |
| 71 | +static CommandResult GetPropertiesFast(string? filePath, JObject command) |
| 72 | +{ |
| 73 | + // ONLY use SW API if THIS SPECIFIC FILE is already open in SolidWorks |
| 74 | + // This prevents loading component files into SW when reading assembly properties |
| 75 | + if (_swApi != null && !string.IsNullOrEmpty(filePath) && _swApi.IsFileOpenInSolidWorks(filePath)) |
| 76 | + { |
| 77 | + Console.Error.WriteLine($"[Service] File is open in SolidWorks, using SW API: {Path.GetFileName(filePath)}"); |
| 78 | + return _swApi.GetCustomProperties(filePath, command["configuration"]?.ToString()); |
112 | 79 | } |
113 | 80 |
|
114 | | - const result = await window.electronAPI?.solidworks?.setProperties(filePath, props, configName) |
115 | | - if (result?.success) { |
116 | | - addToast('success', `Saved tab number to ${configName}`) |
117 | | - } else { |
118 | | - addToast('error', `Failed to save tab number: ${result?.error || 'Unknown error'}`) |
| 81 | + // Use Document Manager API - fast and doesn't load files into SW |
| 82 | + if (_dmApi == null) |
| 83 | + { |
| 84 | + return new CommandResult { Success = false, Error = "Document Manager not available" }; |
119 | 85 | } |
120 | | - } catch (err) { |
121 | | - log.error('[ConfigHandlers]', 'Failed to write config tab to SW file', { filePath, configName, error: err }) |
122 | | - addToast('error', 'Failed to save tab number to file') |
123 | | - } |
124 | | -}, [addToast]) |
| 86 | + |
| 87 | + Console.Error.WriteLine($"[Service] Using Document Manager API for: {Path.GetFileName(filePath)}"); |
| 88 | + return _dmApi.GetCustomProperties(filePath, command["configuration"]?.ToString()); |
| 89 | +} |
125 | 90 | ``` |
126 | 91 |
|
127 | | -### Key Changes |
128 | 92 |
|
129 | | -1. Make both functions `async` |
130 | | -2. After updating the store, call `setProperties()` to write to the SW file's configuration |
131 | | -3. Show toast on success ("Saved to file") and on failure ("Failed to save to file") |
132 | | -4. If the write fails, the pending metadata is still saved and will sync on check-in |
| 93 | +**Important:** Remove the `IsSolidWorksRunning()` check that forces SW API usage. |
133 | 94 |
|
134 | | -### Note on Tab Number |
| 95 | +### Step 2: Fix Response ID Matching |
135 | 96 |
|
136 | | -For tab number, we also need to update the `Number` property (combined base + tab). The TODO notes that we should use serialization settings for the separator, but for now a simple dash works. |
| 97 | +In `Program.cs`, ensure `RequestId` is always set on the CommandResult before returning: |
137 | 98 |
|
138 | | -## Testing |
139 | | - |
140 | | -### Test 1: Config Description |
141 | | - |
142 | | -1. Open a part with configurations in BluePLM |
143 | | -2. Check out the part |
144 | | -3. Edit the description on one of its configuration rows (e.g., T200X) |
145 | | -4. Click away from the input (blur) or press Enter |
146 | | -5. Right-click on a drawing that references that configuration |
147 | | -6. Click "Sync Metadata" |
148 | | -7. **Expected**: Drawing's description should update to the new value immediately |
149 | | - |
150 | | -### Test 2: Config Tab Number |
| 99 | +```csharp |
| 100 | +var result = action switch { ... }; |
| 101 | +result.RequestId = requestId; // Always set before returning |
| 102 | +return result; |
| 103 | +``` |
151 | 104 |
|
152 | | -1. Open a part with configurations in BluePLM |
153 | | -2. Check out the part |
154 | | -3. Edit the tab number on one of its configuration rows |
155 | | -4. Click away from the input (blur) or press Enter |
156 | | -5. Check the SW file properties (or right-click drawing > Sync Metadata) |
157 | | -6. **Expected**: The Number property should show the updated base-tab combination |
| 105 | +### Step 3: Apply Same Fix to Other Fast Operations |
158 | 106 |
|
159 | | -### Verify SW File Updated |
| 107 | +Review and apply the same API selection logic to: |
160 | 108 |
|
161 | | -After editing, you can verify the SW file was updated by: |
| 109 | +- `GetConfigurationsFast` |
| 110 | +- `GetReferencesFast` |
162 | 111 |
|
163 | | -- Opening the part in SolidWorks and checking custom properties |
164 | | -- Or running "Sync Metadata" on a drawing referencing that config |
| 112 | +These should also only use SW API when the specific file is open. |
0 commit comments