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

Admittance/impedance marker data management at Smith chart diagrams #701

Merged
merged 5 commits into from
Oct 10, 2017

Conversation

andresmmera
Copy link
Contributor

So far, when placing a marker at a Smith chart diagram (both impedance and admittance types), the markers always show S-par and impedance data. In this sense, this PR fixes this and includes a new option in marker dialog so as to let the user decide which of these parameters (impedance, admittance, both or none) wants to see.

selection_001

The default behavior is to show impedance data at conventional Smith charts and admittance data at admittance Smith chart diagrams.

It was added two extra fields ('display admittance' and 'display impedance') to the format employed to save the marker settings on the .dpl file.

I'll try to implement this feature in #407 soon. After that, if #407 gets merged, this PR can be closed.

@andresmmera andresmmera added this to the 0.0.20 milestone Jul 22, 2017
@andresmmera andresmmera mentioned this pull request Jul 23, 2017
@ra3xdh ra3xdh mentioned this pull request Jul 24, 2017
11 tasks
@andresmmera
Copy link
Contributor Author

I've tested 428b687 and it's fine.
admittancestuff

@in3otd
Copy link
Contributor

in3otd commented Aug 4, 2017

I've added the options to show also the series and parallel equivalent components values:

export

@andresmmera
Copy link
Contributor Author

Reviewed and tested locally. It works fine.
eqcktfeature

However, I think I've found a bug... When the marker is about to cross the inductive/capacitive border it jumps to the center of the Smith chart, it stays there for a while and then it continues following the curve trace (it's quite weird...). Please, see the following screencast:

https://drive.google.com/open?id=0BwDjrwGfd_z7SFYxUHYtNFBrdVU

Notice that the marker reading is correct though...
This issue is not related to this PR, but it also happens at the master branch...
I'll try to see what's going on here...

@in3otd
Copy link
Contributor

in3otd commented Aug 5, 2017

thanks for the fast review, while testing I saw the same issue and fixed it after pushing to this PR. I was not sure if it should go to a separate PR, it's just a small thing here; the +1 should have been added later to R, if at all. I'll push the change so you can take a look.

@guitorri guitorri self-requested a review October 9, 2017 17:56
andresmmera and others added 5 commits October 9, 2017 20:37
was recently pointed out at qucs-devel mail list that this makes little
sense. In this commit, this behaviour is fixed so that markers show
impedance/admittance data at impedance/admittance Smith charts by
default.
to the Smith charts marker optional text.
Reworked the marker text options handling to keep them
in a single variable/field.
values for the Smith charts markers
used e.g. to check whether the marker pointer is inside a chart and so should
be drawn or just point to the diagram center.
The current code was not correct for locations very close to the circle boundary.
@guitorri guitorri force-pushed the AdmittanceDataFeature branch from 274c59a to 86b343f Compare October 9, 2017 19:25
Copy link
Member

@guitorri guitorri left a comment

Choose a reason for hiding this comment

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

Looks good. Rebased and waiting CI to complete and merge.

@guitorri guitorri merged commit 1e7fd62 into Qucs:develop Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants