Addition of the Wattmeter and Ohmmeter Modules

[gildias]

Hello, my name is Alberto Silva and I'll be continuing the module development started by previous students from my institution (ISEP, Portugal).
This commit is related to the watt probe. Since this is my first interaction with github, I'd appreciate some information on whether I did the pull process correctly, and if not, what needs to be changed.
I'll likely be sending some more modules over the next weeks so I would like to know if I should keep developing over my existing files ( with the wattmeter in them ) or if I should start over for each module.
This is important since some modules will share modified files, with the obvious example of a makefile.
I have also added a simple pdf with 3 examples of the wattmeter working, hope it was attached correctly.
Thank you for your time.
Edit: I did not know you would receive the comment I made when i originally pushed to my git branch, thought this was the commit message, sorry about that.

Wattmeter examples.pdf

[andresmmera]

I reckon that you pushed correctly :-)
I'm going to take a look to it. Good job

[andresmmera]

Just a few comments, I promise I'll do some more revision later:

• There are two typos at 'qucs-core/src/components/wprobe.cpp'. 'Potency Factor' is wrong, use 'Power Factor' instead

• I think that it would be more convenient to use the following notation since it seems to be more common:

Parameter	Meaning
Q	Reactive power
P	Real power
S	Apparent power
PF	Power factor

• The power factor is defined as PF = cos(alpha), so in the following line https://github.com/Qucs/qucs/pull/682/files#diff-dac894bc3f01dacc2d74d3d56f4a3b1aR78 I think you can remove line 77 and put 'setOperatingPoint ("PF", cos(alpha));' Probably, it would be better to use 'PF' (Power Factor) instead of 'FP' (factor de potência?)

• It would be nice if you could add a comment before the function instance at https://github.com/Qucs/qucs/pull/682/files#diff-dac894bc3f01dacc2d74d3d56f4a3b1aR63 pointing to some document (book, notes or similar) just to let those who eventually see the code where to find the equations.

[1] https://en.wikipedia.org/wiki/Power_factor#Definition_and_calculation

[guitorri]

Just to mention that this a cleanup of #546.

[guitorri]

@gildias concerning your questions:

Yes your PR is fine. You can amend your commit message and push the branch once again. Something like:

cd ~/git/qucs/
git checkout wattmeter
git commit --amend # edit last commit message and save
git push -f [your origin] wattmeter

This will force update your fork/branch and this PR.

Indeed if you branch from develop for each new module the makefiles might create conflicts.

You could branch from the previous module for each new modules (wattmeter->moduleX then moduleX->moduleY ...) and we could merge things in this order just the last PR.

Perhaps is easier for you and for the review if you try to make one commit per module in this PR. So, I suggest you keep working on your wattmeter branch, one module per commit.

Please prepare a few example or test schematics for each module so we can add later to the test suite.

[andresmmera]

Maybe I'm just nitpicking, but I think you can calculate the power factor as follows (PF = P/|S|):

setOperatingPoint ("FP", Wr/std::sqrt(WrWr+VAiVAi));

that way you can avoid the std::cos() and std::atan() functions

Notice that |S| = std::sqrt(Wr * Wr+VAi * VAi) = std::sqrt(P^2 + Q^2) and P = Wr (real power), Q = VAi (reactive power)

[gildias]

Thank you for all the precious information you provided.
Edited the commit according to your comments, now that I know how the process works, I'm sure it will go smoothly in the future.
If you have any additional concerns or advice regarding this commit, I'll be happy to address them.

I would appreciate some clarification on the subject of the test suite. I have the circuit schematics where the wattmeter was tested (as seen on the pdf file), and will add new examples as I progress through other modules, should i add these to the commit on a particular folder (qucs-test)?.

I would also like to know how I should approach adding a new commit to this pull request, If I follow the same process i did initially, am I given the option of creating a new commit instead of editing this one? Maybe that is the default behaviour and there is nothing to worry about, but I would welcome some confirmation.

Thank you for your time and consideration.

[gildias]

Pursuing the objective of helping students or beginners interested in electronics, I have now added the second commit, the Ohmmeter Module. It calculates the equivalent impedance in a given circuit.
If you have any advice or see anything that could be improved, I appreciate the information and will try to do so.
I would like to add the example schematics but I am still unsure on how I should approach that, so some clarification would be helpful.
Attached a pdf with working examples of the ohmmeter as well.
Thank you for your time and attention.
Ohmmeter examples.pdf

[gildias]

Resistance examples.pdf
Added the parameter of internal resistance to IProbe, Vprobe, Wprobe, Idc, Iac, Vdc and Vac.
The objective of this new function was to easily allow the study of the influence a real component can have on an electronic circuit.
As always, I will do my best to implement any feedback or recommended changes.
Regarding the question i sent to @guitorri email, I will update this commit depending on your answer, Andrés graciously helped with the other technical problems I was having.
Thank you for your time, and particularly to @andresmmera for his crucial help on the implementation of the voltmeter.

[andresmmera]

You're welcome 😃
I've just tested (and reviewed the changes of) this last commit and they look good 👍

[gildias]

Added the phasorial and temporal digrams.
The phasorial diagram represents the data of a circuit on a complex plane. It can display more than one frequency at once, giving the option of easily analysing the effect of a different frequency on various parameters of the circuit, such as Power, Impedance, Current and Voltage. It is also helpful in determining whether a circuit is resistive, capacitive or inductive.
The temporal diagram acts like a basic oscilloscope, showing the evolution of a wave in relation to time, allowing for a simple analysis of the ofset between a number of waves.
The frequencies can be set on the diagrams themselves, as long as they have been simulated already, giving the option of simulating more than one frequency without running multiple simulations.
Both of these diagrams can be used exclusively on AC simulations.
If you have any comments, I will try to implement any helpful feedback.
Thank you for your time.
Diagrams examples.pdf

[gildias]

Hello, I probably won't be adding any other extensive commits to this pull request.
Therefore, i would appreciate it if an analysis could be made so that these new modules can be integrated into the official release. I think these additions would be extremely useful, especially for those interested in learning electronics. Your feedback would also be extremely helpful in regards to my project presentation.
I will do my best to answer any questions or comments that might occur, and will apply any changes that are deemed necessary.
Thank you for your time and consideration.

[gildias]

Hello, I apologise for insisting on this subject, but with the presentation date for my project getting closer, I would really appreciate some feedback from the QUCS team.
@andresmmera, as the person that helped me the most during development, could you possibly take a look at the PR as a whole and begin the testing process?
I completely understand if you're not available for any reason but I believe these modules will be a good addition to the QUCS comunity.
Thank you for your time and consideration.

[andresmmera]

Hello @gildias,
I'll try to review the whole PR in (more or less) depth ASAP (I'll need maybe the weekend). In my opinion, this PR is a good one and for sure it'll add some value to the Qucs project.
Please notice that I'm not the guy making decisions about what PRs should be merged or not... It may take several months (or even years!) before someone merges a PR, so don't expect this PR is going to be merged soon.

[gildias]

I completely understand, if you have any questions or need any more files, such as test schematics to simplify the process for you, I'll make sure to help however I can. You can contact me through this thread or my email 1120196@isep.ipp.pt .
Thanks again.

[andresmmera]

Hello @gildias ,

I've been doing some checking stuff here and I've found the following:

1. Temporal diagram
   1. The aspect ratio of the temporal diagram cannot be changed. Take for example a Cartesian plot, you can modify the default aspect ratio easily with the mouse. I think that temporal diagrams should behave like this too.
   2. Minor thing. I reckon that it would be more meaningful if you called it 'AC Temporal diagram'. Otherwise, it may mislead some user to think that this diagram could be used for other simulations.
   3. Qucs crashes when the the AC analysis is performed over a single freq. It seems that you have some problem with the length of a vector (I didn't have time to dig on that)
      It throws this message:

terminate called after throwing an instance of 'std::length_error'
what(): vector::_M_default_append
Aborted (core dumped)
See #691

2. Phasor diagram

   1. It doesn't work when the AC analysis is performed over a single frequency.
   2. I've noticed you have to specify the freq using a QLineedit. This way the user can enter a frequency not present in the AC analysis. Did you consider using a QCombobox instead?
   3. The aspect ratio of the diagram cannot be changed too but I think that this is not so important here.

3. Power probe component
   In the properties of the component you can specify the internal resistances of the voltage and current probes. I think that it would be clearer if you made explicit reference to that in the description field. The current properties look like this,

Name	Value	display	Description
Riv	0 Ohm	no	Internal resistance, (0: disable), ideal behaviour
Rii	0 Ohm	no	Internal resistance, (0: disable), ideal behaviour

here we understand what Riv and Rii mean, but it may be not so clear for a random user. From my point of view, the description should explicitly describe what the parameter is. I mean, for example, the description of the 'Riv' parameter should be something like "Voltage probe internal resistor". I also think that is not necessary to clarify that 0 Ohm is the ideal behavior.

4. Wattmeter component
   I've been checking again the wattmeter component and it is fine, but I noticed that the apparent power (S) is given as a complex number whereas it is usually S = abs(S). Take a look at the last example here

5. Ohmmeter
   I think that the Ohmmeter performance can be checked using this schematic Here the input impedance of a filter is calculated using the Ohmmeter (AC analysis) and the S-parameter simulation. Both analyses give the same results, so it proves that the Ohmmeter works.

6. Annoying warning (don't worry, it is not your fault)

The terminal output shows the following message:

'Warning: QLayout: Attempting to add QLayout "" to SaveDialog "", which already has a layout'

This stuff is related to this 2e9f7d1#commitcomment-21969888 A fix for that was made by @in3otd. Actually, this fix is already merged on the develop branch (deb4663), but your wattmeter branch seems to be behind develop. That is, your branch and develop have diverged since the day you've cloned your branch because people have merged stuff in develop and you've been working on your branch. So you have to bring these new changes from develop to your branch. In order to do this, you have to rebase your branch against develop:

git checkout wattmeter
git rebase develop

You may find a number of conflicts that cannot be solved automatically, so you'll have to solve them by hand... I usually do that using program called meld Once you fix a conflict, you have to mark it as resolved and do:

git rebase --continue

... and you have to repeat this step until you solved all conflitct. Then, you have to commit the changes and push as usual.

Apart from these issues, I consider the code is mergable and adds value to the AC simulation. Well done!

[guitorri]

So, the people form ISEP want this code merged and hopefully released soon.
I don't have time these days to do review. @andresmmera you are in the best position to judge if the code is good to be merged. If so, tag it for the next milestone and merge.
You don't have to wait for one guy to decide and merge stuff into develop. The release branch is another story, and a bit of coordination might make sense.
It would be nice to have a few examples added to qucs-test later on.

[andresmmera]

From my point of view, the code is worthwhile merging since it adds new capabilities to the AC analysis.
The issues I've posted here before are not a must, but, honestly the items 1.iii) and 2.i) should be solved before merging in order to guarantee proper functionality.