jankae / LibreVNA

100kHz to 6GHz 2 port USB based VNA
GNU General Public License v3.0
1.13k stars 210 forks source link

widgets/charts: implement polar diagram #144

Closed sophiekovalevsky closed 2 years ago

sophiekovalevsky commented 2 years ago

The following implementation represent Sii parameters in a polar chart.

I would like to receive your initial feedback on this. During development I came up with the following thoughts:

jankae commented 2 years ago

Wow, thank you so much for this addition. I haven't looked at it in detail yet but my initial thought is to go with a common base class for smith and polar charts as there are only a couple difference between them (mainly the grid lines and pixel<->coordinate conversions.

sophiekovalevsky commented 2 years ago

@jankae ready for review.

jankae commented 2 years ago

Thank you very much :) I am unavailable for a few days and will review/merge this in about a week.

jankae commented 2 years ago

Merged, and thanks again for this significant contribution :)

I made slight changes:

It's desirable to apply this chart to parameters not equal to Sii?

I think so and am allowing Sij traces to be added to polar charts now

Trace points when are out of visible area may have some empty spaces near edgeReflection circle because there is no interpolation between the sample point and the next one. This can be exacerbated if zoom factor is higher.

Improved handling of line drawing in smith and polar charts. The line is now drawn all the way to the circle in case one point is outside of it.

sophiekovalevsky commented 2 years ago

@jankae you're welcome.

Improved handling of line drawing in smith and polar charts. The line is now drawn all the way to the circle in case one point is outside of it.

I haven't look thoroughly the changes but while making some tests, I discovered the following:

image

Is this an expected result?

jankae commented 2 years ago

Hm, looks like I forgot to check this "improvement" when an x-axis offset is used... Will soon be fixed

jankae commented 2 years ago

Should be fixed now, I forgot to handle a special case in constrainLineToCircle

sophiekovalevsky commented 2 years ago

@jankae it works sooo great. Thanks.