search

jankae / LibreCAL

4 port eCal module
GNU General Public License v3.0
60 stars 19 forks source link

LibreCAL Improvements #2

Closed bvernoux closed 1 year ago

bvernoux commented 1 year ago

Thanks for that amazing project see the improvements I have done:

jankae commented 1 year ago

Thank you so much, especially for fixing the warnings in KiCad (I am probably not up to speed on that one).

I only have a couple of questions before I merge this:

Improved Impedance for JLCPCB 4Layer JLC7628 1.6mm

As far as I can see, you changed the trace width from 0.3mm to 0.295mm. Or did I miss anything else? Is that an improvement based on actual measurements or "just" on calculations? I have gotten good results with the 0.3mm trace so unless you actually measured an improvement, I am hesitant to change it. Then again, this is such a small change compared to the FR4 tolerance that it probably won't make a significant difference.

https://github.com/jankae/LibreCAL/blob/d16876c3797856dbade114fa01f14dd21224174a/Software/LibreCAL-GUI/LibreCAL-GUI.pro#L3

Your PR adds the 'svg' module to Qt. What is this about? At the moment it makes the CI for Ubuntu fail but I guess this could be solved by installing that module before compiling. I just don't see anything in your changes that would require the module in the first place.

bvernoux commented 1 year ago

Thank you so much, especially for fixing the warnings in KiCad (I am probably not up to speed on that one).

I only have a couple of questions before I merge this:

Improved Impedance for JLCPCB 4Layer JLC7628 1.6mm

As far as I can see, you changed the trace width from 0.3mm to 0.295mm. Or did I miss anything else? Is that an improvement based on actual measurements or "just" on calculations? I have gotten good results with the 0.3mm trace so unless you actually measured an improvement, I am hesitant to change it. Then again, this is such a small change compared to the FR4 tolerance that it probably won't make a significant difference.

Yes it is a very small change which do not impact a lot especially for 0.005mm but I have also rounded the track (which is always better for RF even if that shall not impact a lot too) and I have created a Net Class => RF_Z50Ohms for RF signals which requires 50 Ohms Impedance traces to easily change the width of those traces for an other process (with different height/Er...)

https://github.com/jankae/LibreCAL/blob/d16876c3797856dbade114fa01f14dd21224174a/Software/LibreCAL-GUI/LibreCAL-GUI.pro#L3

Your PR adds the 'svg' module to Qt. What is this about? At the moment it makes the CI for Ubuntu fail but I guess this could be solved by installing that module before compiling. I just don't see anything in your changes that would require the module in the first place.

Yes it is mandatory for VisualStudio 2019 static build as you are using svg which requires to have that explicitely else the svg picture are not displayed (as I suspect the static lib for svg is not linked). It is strange that by adding svg module to Qt the build fail for Linux build so maybe to be added only for Windows build (which pass with success)

jankae commented 1 year ago

Ok, thank you for clarifying. I'll try to fix the Ubuntu workflow and merge