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

Add new s21 markers for |Z| and R+jX #384

Merged
merged 1 commit into from
May 8, 2021

Conversation

akinad3
Copy link
Contributor

@akinad3 akinad3 commented Apr 4, 2021

Adds new markers for
S21 |Z| shunt
S21 |Z| series
S21 R+jX shunt
S21 R+jX series

issue #165

@akinad3 akinad3 requested a review from zarath as a code owner April 4, 2021 21:15
@akinad3 akinad3 changed the base branch from testing to Development April 5, 2021 15:49
@akinad3 akinad3 changed the base branch from Development to testing April 5, 2021 15:49
Copy link
Contributor

@rjordans rjordans left a comment

Choose a reason for hiding this comment

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

Overall this seems to work nicely except for the one comment above about format_complex_imp() and printing negative real values nicely

@@ -351,3 +351,7 @@ def updateLabels(self,
self.label['s21phase'].setText(format_phase(s21.phase))
self.label['s21polar'].setText(
str(round(abs(s21.z), 2)) + "∠" + format_phase(s21.phase))
self.label['s21magshunt'].setText(format_magnitude(abs(s21.shuntImpedance())))
self.label['s21magseries'].setText(format_magnitude(abs(s21.seriesImpedance())))
self.label['s21realimagshunt'].setText(format_complex_imp(s21.shuntImpedance()))
Copy link
Contributor

Choose a reason for hiding this comment

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

The format_complex_imp() will need its optional second argument set to True to properly display negative real numbers here (same holds for the next line)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, ammended the commit.

@zarath zarath merged commit 6be2730 into NanoVNA-Saver:testing May 8, 2021
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