search

sparkmicro / Ki-nTree

Fast part creation for KiCad and InvenTree
GNU General Public License v3.0
171 stars 31 forks source link

1.0.5 #197

Closed eeintech closed 5 months ago

eeintech commented 5 months ago

Fixes: https://github.com/sparkmicro/Ki-nTree/issues/191

eeintech commented 5 months ago

@T0jan I think your PR has introduced something weird as alternate resistors are created as new parts in InvenTree when they shouldn't, I didn't get much time to look into it but I can't reproduce it within the GUI. Any ideas?

T0jan commented 5 months ago

@eeintech I haven't changed anything involved with the alternate test so it has to be some kind of side effect. lets see if I can find anything. It is very weird, that it works for the capacitor but not for the resistors later, so maybe it is something with the configs, not the code.

T0jan commented 5 months ago

@eeintech Ok I found the reason: it is the Temperature Range parameter. As we use the stable branch again the parameter check is active by default (#165) and so the parameter gets rejected. For resistors it is part of the compared parameters, for capacitors not so this explains why they act different.

I don't know if it is possible to change the inventree config during the test or in the startup to disable the parameter check.

eeintech commented 5 months ago

@T0jan This is strange, I cannot reproduce the Temperature Range discrepancy locally, although enabled in my paramaters_filters.yaml config file:

image

Both original and alternate MPNs show those parameters locally:

{'Package Type': '0603', 'Rated Power': '1/10W', 'Temperature Range': '-55~155°C', 'Tolerance': '±5%', 'Value': '10K'}
T0jan commented 5 months ago

@eeintech has the local server you test this with the parameter test enabled? or do you just run the release pipeline locally?

I run the jobs in my fork with debug output and there the discrepancy is clear:

https://github.com/T0jan/Ki-nTree/actions/runs/7529280107/job/20493179309#step:10:1763

eeintech commented 5 months ago

@T0jan It seems like there is now some kind of conversion going on with parameters?

requests.exceptions.HTTPError: {'detail': 'Error occurred during API request', 'url': 'http://127.0.0.1:8000/api/part/parameter/', 'method': 'POST', 'status_code': 400, 'body': '{"data":["Could not convert -55~155°C to °C"]}', 'headers': {'AUTHORIZATION': 'Token inv-b344cee97aceb609bee46f5beab6aced190a3f7e-20240115'}, 'params': {'format': 'json'}, 'data': {'part': 1, 'template': 50, 'data': '-55~155°C'}}

The devil must be in the details because it works on local setup, which means that °C is not equal to °C??

T0jan commented 5 months ago

@eeintech yes there is a conversion, it is the #163 thing which they added for 0.12.0. Inventree by default expects now parameters to be SI-compatible so always something like -50°C, 100mm, 42uF and so on. So a multivalue entry like -55~155°C is not allowed by the server. I told them this is not the best idea so they added a button to be able to switch this off, do you remember?

image

I do not know if it is possible to unset this setting via environmental variables in the server setup.

Also I noticed that my check for exact this setting does not work anymore as they changed the error message. So this is fun...

eeintech commented 5 months ago

@T0jan I'm sorry, this is new setting completely came out of my mind... just too many things going on right now 😓

I do not know if it is possible to unset this setting via environmental variables in the server setup.

Righto I need to follow-up on that... I'm tempted to merge this right now and release 1.0.5 with all the changes then sort this out. If that setting is enabled by default on new and older InvenTree setups, it will cause duplicate resistors to be created (eg. rendering the "Check Existing" toggle useless). What do you think?

T0jan commented 5 months ago

@eeintech No worries, I should have written it clear right away, would have saved you the debug time...

I don't think anything speaks against releasing it as is per se, as the setting is documented already it should not cause to much problems for users and not a problem with Ki-nTree. However I would like to patch the error detection for this specific case, so the user gets a clear message if they did not disabled this setting.

If that setting is enabled by default on new and older InvenTree setups, it will cause duplicate resistors to be created (eg. rendering the "Check Existing" toggle useless). What do you think?

I don't think so. It only gets useless if the setting is not deactivated and the part parameters include a non convertible value. The default case should be the deactivated case where all parameters are generated as before (That's why we added it to the documentation) and there the existing part check will work as before.