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 sorting: update cached part stock quantity if a mismatch with db is found #447

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gyohng
Copy link
Contributor

@gyohng gyohng commented Apr 21, 2024

This patch corrects a sorting issue in the application where the GUI displayed the correct stock quantities but used outdated cached values for sorting operations.

The fix targets the populate_footprint_list method and involves a condition that checks if the cached stock value (part[5]) in the GUI matches the most recent stock quantity from the database (detail[0][1]). If a mismatch is detected, the cached value is updated to the current database value, and also set_stock is called, which updates the current board database with the new stock values.

part[5] = detail[0][1]
if part[5] != str(detail[0][1]):
part[5] = str(detail[0][1])
self.store.set_stock(part[0], detail[0][1])
Copy link
Collaborator

Choose a reason for hiding this comment

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

when would this condition occur? When the database is updated while the plugin is open, without closing and reopening the plugin?

Copy link
Contributor Author

@gyohng gyohng Apr 22, 2024

Choose a reason for hiding this comment

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

It will occur persistently after updating a database while having some projects with LCSC components filled from before the update. Reopening the plugin doesn't help. self.store is project-bound and does not explicitly sync its stock quantity value with the database at all. There's just one path where it does - and that's when you use a part selector - and never afterwards. This fix is meant to address it. The condition is there not to call set_stock always, which works through sqlite.

If this code is not there, the sorting logic does not use the data from detail and only considers the non-overridden old once cached and never updated data inside part[5]

…se the cached field is used for sorting directly.
@gyohng gyohng force-pushed the fix--update-part-stock-quantity branch from 163ccfb to 4962786 Compare July 3, 2024 08:54
@gyohng
Copy link
Contributor Author

gyohng commented Jul 3, 2024

Rebased to the main branch. Please consider merging.

@Bouni
Copy link
Owner

Bouni commented Jul 4, 2024

I'm on vacation but will be back on Monday. Then I look into it

@Bouni
Copy link
Owner

Bouni commented Aug 5, 2024

I think the get_stock method is no longer around so having set_stock is useless.
We should stop of saving the stock value to the project db and drop that field as we get the stock dircetly from the parts db every time we populate the footprint list.

Thoughts? @gyohng

@Bouni
Copy link
Owner

Bouni commented Aug 5, 2024

I just realized that we have the stock in the project db so that we can abuse the database for sorting the parts list 🤦🏽‍♂️

This is far from ideal, ideas how we could get sorting better than writing "usless data" into the project db?

@gyohng
Copy link
Contributor Author

gyohng commented Aug 5, 2024

What do you think about populate_footprint_list calling get_part_details to retrieve stock from the main db?

We could also decorate get_part_details with @lru_cache with a cache size of 100000 (large enough to cover almost any BOM, and small enough to prevent memory overflows). Things get accessed from the main database anyway, so once it's called, it'll cache the data, and repeated calls to the same function will just return a cached value.

The download logic can call get_part_details.cache_clear() to reset this cache.

Here's more info:
https://www.geeksforgeeks.org/python-functools-lru_cache/

@Bouni
Copy link
Owner

Bouni commented Aug 5, 2024

We already do that: https://github.com/Bouni/kicad-jlcpcb-tools/blob/main/mainwindow.py#L608

I don't think a cache is necessary, as the search is quite fast (I have not tested this with huge projects but I asume that < 1000 parts is the most likely case).
A cache would bring additional complexity and would not benefit the average user.

I did a few test and found out that ther's a bug in the sorting anyway, but even when I fix the sorting the sort by stock does not work as expected and I don't know why, all other columns wor as expected ...

@gyohng
Copy link
Contributor Author

gyohng commented Aug 5, 2024

Can you submit what you have, I'll check if I can chase the stock sorting

@Bouni
Copy link
Owner

Bouni commented Aug 6, 2024

@gyohng Not much yet but I made a PR for it: #517

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.

3 participants