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

S-parameter viewer #936

Merged
merged 8 commits into from
Sep 15, 2024
Merged

S-parameter viewer #936

merged 8 commits into from
Sep 15, 2024

Conversation

andresmmera
Copy link
Contributor

@andresmmera andresmmera commented Sep 8, 2024

I have added an S-parameter viewer to the Qucs-S toolset. The idea originated in #733 with the approval of ra3xdh.

The goal of this tool is to be able to read and display S-parameter data (for now, magnitude only). Basically I have ported to CPP the code from [1] and added some features like the ability to open n-port files and have data markers.

The tool is accessible from the Tools menu. It has a single window divided into three panels: one on the right (chart) and two stacked panels on the left (axis settings, file management, trace management and markers).

MainWindow

S-parameter files can be opened by dragging and dropping them onto the S-parameter files area. For 2-port files, the tool automatically displays S21, S11 and S22. It also automatically calculates important metrics for 2-port devices such as K, mu, MAG, etc.

If several s2p files are added on a row, it shows all the S21 traces.

The user can add data markers. Trace data is displayed in a table

Selection_036

This is an screencast showing how it works:

https://drive.google.com/file/d/1tMzdM6z_twqlb8ezXjk8dJaYi-Dm-o-e/view?usp=drive_link

I have tested a number of S-parameter files. I collected a few examples in this spreasheet. Please find in [2] the test workspace I used for this.

[1] https://rfdesigntools.pythonanywhere.com/tool/s-parameter-viewer
[2] SPAR Viewer Testbench_prj.zip

This commit contains the first draft of the user interface
Read Touchstone files

It was implemented a basic function to read Touchstone files.

It can only read Touchstone files up to 4 ports and only S-parameter data.


Please see "Touchstone Specification, Version 2.1, ratified January 26 2024 by the IBIS Open Forum": https://ibis.org/touchstone_ver2.1/ 
Add QScrollArea widgets to the files and traces lists

Large number of files and traces are expected, so there is a need for scrollable areas in the files and traces lists.
Basic plotting structure


Add default behaviour when loading one single s2p

A default behavior is added. When a single s2p file is selected, the program automatically displays S21, S11 and S22
Replace the "Delete" message by a trash image

The delete image was taken from here

https://commons.wikimedia.org/wiki/File:Delete-button.svg

This file is licensed under the Creative Commons Attribution-Share Alike 4.0 International license.

    You are free:

        to share – to copy, distribute and transmit the work
        to remix – to adapt the work

    Under the following conditions:

        attribution – You must give appropriate credit, provide a link to the license, and indicate if changes were made. You may do so in any reasonable manner, but not in any way that suggests the licensor endorses you or your use.
        share alike – If you remix, transform, or build upon the material, you must distribute your contributions under the same or compatible license as the original.
Read Touchstone files with more than two ports


Update traces combobox depending on the selected dataset

If the user has loaded data with different number of ports, the traces combobox must be refreshed each time the user changes the dataset selection. Otherwise, this may cause that the user selects a non existing trace
Fix style in buttons for removing files

The QPushButtons were replaced by QToolButtons. With the QPushButtons the widget was too wide
Fix style in buttons for removing traces

QPushButton was converted into QToolButton
Delete dataset and its traces when the user decides to remove a file


Remove files and traces

Rework on the logic on how to remove datasets and traces
After removing a file, remove the associated widgets


Remove trace from Chart


Update file widget position in the grid after removing


Function handler for changing the color of a trace


Change linestyle depending on the combo selection


Set initial color of the color pickers


Added a spinbox control to control the trace width

It was added a spinbox that controls the width of the traces displayed. This is very convenient when a bunch of traces are being displayed and the user wants to highlight one of them easily
Added function  handler for controlling the x-axis

A handler function was added to control the x-axis settings as the user changes the minimum, maximum or the tick interval
Update traces when changing the axis settings


Fix trace plotting refresh


Fix frequency limits when loading a GHz range file


Dockable widgets


Autoadjust y-axis settings


Automatically add K, delta, mu_s, m_p, MAG and MSG traces in S2P files

When a Touchtone file has two ports, the stability metrics are automatically computed and added to the dataset
Add marker table feature

It was added a new dock consisting on a marker table and some widgets for its management
Add dot marker and vertical lines in the QChart


Make case insensitive the frequency scale

Files were found were the frequency scale is all in capital letters. This creates a problem when reading the spar data. This commit fixes this by putting the frequency scale in lower case
Auto adjust x-axis when changing the units


Put x_div values as a ComboBox

It makes no much sense in having a decimal spinbox for defining the tick interval. It leads to decimal ticks. It's better to have a closed list of possible values
y axis tick in combobox


Fix vertical line markers


Fix bad "About Qt" connection 

The "About Qt" message was not properly connected. As a consequence, when the user went to "Help-> About Qt..." nothing showed up.

This commit is intended to fix this by connecting the menu with the handler as it's done in the filter design tool
Link S-parameter viewer to Qucs-S


Add Re{Zin}, Im{Zin} traces to s1p and s2p files


Hide y-axis units

It makes no much sense for now to have it since it may happen that the y-axis represent dB, Ohm or simply its unitless (e.g. K, mu, ...)
Fix segfault when removing one single file

In previous commits, it was observed a segfault when removing one single s-par file. This happened because the program was freeing widgets already freed. This situation is avoided by ordering the list of widgets to remove
Autoadjust y-axis


Remove widgets for marker placement

They are actually not needed. The SpinBox and the combo with units just add clutter. The user can set the marker freq once added
Update x-axis limits after removing file


Increase maximum x-ticks


Get suffix using Qt method

This is more robust than the previous approach
Fix frequency scale in markers


Enable drag and drop to open files


Fix segfault when removing file


Readjust frequency limits when dataset has no traces


Fix read touchstone

Files were found whose header contains no !
Fix initial marker step


Fix autoscale y-axis


Prevent docks from closing

It makes no sense the user can close the docks
Solve infinite loop when fmax=3000 [unit]


Implemented button for removing all files on a row


Implement button for removing all markers on a row
@andresmmera
Copy link
Contributor Author

Any guess on how to fix this?

image

@ra3xdh ra3xdh added this to the 24.4.0 milestone Sep 8, 2024
@ra3xdh
Copy link
Owner

ra3xdh commented Sep 8, 2024

Thanks for the contribution! I will evaluate the new tool and report the feedback. The most probably I will merge this after preparing v24.3.1 package. Or maybe I will cancel the 24.3.1 release and make 24.4.0 including a new tool instead.

Any guess on how to fix this?

The qtcharts should be added in .github/workflows/deploy.yml according this guide: https://github.com/jurplel/install-qt-action

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 8, 2024

The CI still failing but another error appears:

No rule to make target '/home/vvk/qucs_s/qucs-s-spar-viewer/qucs-s-spar-viewer.hspar_viewerf', needed by 'qucs-s-spar-viewer/moc_qucs-s-spar-viewer.cpp'.  Stop.

I am getting the same error when trying to build a new tool locally. This is not related to CI. I guess some files may be presented locally but missing in commit.

@andresmmera
Copy link
Contributor Author

I see. I'll check that later.

@andresmmera
Copy link
Contributor Author

I am getting the same error when trying to build a new tool locally. This is not related to CI. I guess some files may be presented locally but missing in commit.

You need to install "libqt5charts5-dev".

I set up a fresh Lubuntu 24.04 in a VM and followed the instructions you provide in README:

sudo apt-get install ngspice build-essential git cmake qtbase5-dev qttools5-dev libqt5svg5-dev flex bison gperf dos2unix
git submodule init
git submodule update
mkdir builddir
cd builddir
cmake ..  -DCMAKE_INSTALL_PREFIX=/your_install_prefix/
make
make install

After installing "libqt5charts5-dev" it went fine.

I'll continue tomorrow with this stuff.

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 8, 2024

You need to install "libqt5charts5-dev".

I am using Qt6. Bot qt5-chart and qt6-charts development version are installed on my machine. Need further investigations.

@dsm
Copy link
Collaborator

dsm commented Sep 9, 2024

I tried to include Qtcharts like this #include <QtCharts/QtCharts>

and CI compiler give error this line using namespace QtCharts; QtCharts is not a name space name

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 9, 2024

and CI compiler give error this line using namespace QtCharts; QtCharts is not a name space name

See this note: https://doc.qt.io/qt-6/qtcharts-changes-qt6.html Some version-depedent code is needed here.

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 9, 2024

@andresmmera Try to apply the following patch. It fixes the Qt6 build. Now I am able to build the tool locally using the Qt6.7.2 on Linux.

diff --git a/qucs-s-spar-viewer/qucs-s-spar-viewer.cpp b/qucs-s-spar-viewer/qucs-s-spar-viewer.cpp
index 75b6dacd..79d53c33 100644
--- a/qucs-s-spar-viewer/qucs-s-spar-viewer.cpp
+++ b/qucs-s-spar-viewer/qucs-s-spar-viewer.cpp
@@ -39,7 +39,7 @@
 Qucs_S_SPAR_Viewer::Qucs_S_SPAR_Viewer()
 {
 
-  QWidget *centralWidget = new QWidget(this);  
+  QWidget *centralWidget = new QWidget(this);
   setCentralWidget(centralWidget);
 
   setWindowIcon(QPixmap(":/bitmaps/big.qucs.xpm"));
@@ -560,7 +560,7 @@ void Qucs_S_SPAR_Viewer::addFiles(QStringList fileNames)
           //qDebug() << line;
 
            if (line.isEmpty()) continue;
-           if ((line.at(0).isNumber() == false) && (line.at(0) != "#")) {
+           if ((line.at(0).isNumber() == false) && (line.at(0) != '#')) {
                if (file_data["frequency"].size() == 0){
                    // There's still no data
                    continue;
@@ -572,7 +572,7 @@ void Qucs_S_SPAR_Viewer::addFiles(QStringList fileNames)
 
            }
            // Check for the option line
-           if (line.at(0) == "#"){
+           if (line.at(0) == '#'){
 
                QStringList info = line.split(" ");
                frequency_unit = info.at(1); // Specifies the unit of frequency.
@@ -1443,7 +1443,7 @@ void Qucs_S_SPAR_Viewer::updateTraces()
         QString data_file = trace_name_parts[0];
         QString trace_file = trace_name_parts[1];
 
-        if (trace_file.at(0) == "S"){
+        if (trace_file.at(0) == 'S'){
             trace_file = trace_file + QString("_dB");
         }
         if (trace_file == QString("|%1|").arg(QChar(0x0394))){
@@ -1603,7 +1603,7 @@ void Qucs_S_SPAR_Viewer::checkFreqSettingsLimits(QString filename, double& fmin,
                 disconnect(QCombobox_x_axis_units, SIGNAL(currentIndexChanged(int)), this, SLOT(updatePlot())); // Needed to avoid duplicating the call to the update function
                 disconnect(QSpinBox_x_axis_max, SIGNAL(valueChanged(int)), this, SLOT(updatePlot())); // Needed to avoid duplicating the call to the update function
                 disconnect(QSpinBox_x_axis_min, SIGNAL(valueChanged(int)), this, SLOT(updatePlot())); // Needed to avoid duplicating the call to the update function
-                QCombobox_x_axis_units->setCurrentIndex(index+1);               
+                QCombobox_x_axis_units->setCurrentIndex(index+1);
                 QSpinBox_x_axis_min->setValue(fmin);
                 QSpinBox_x_axis_max->setValue(fmax);
                 connect(QCombobox_x_axis_units, SIGNAL(currentIndexChanged(int)), SLOT(updatePlot()));
@@ -1643,7 +1643,7 @@ void Qucs_S_SPAR_Viewer::checkFreqSettingsLimits(QString filename, double& fmin,
 void Qucs_S_SPAR_Viewer::adjust_y_axis_to_trace(QString filename, QString tracename){
     qreal minX, maxX, minY, maxY;
 
-    if (tracename.at(0) == "S"){
+    if (tracename.at(0) == 'S'){
         tracename = tracename + QString("_dB");
     }
     if (tracename == QString("|%1|").arg(QChar(0x0394))){
diff --git a/qucs-s-spar-viewer/qucs-s-spar-viewer.h b/qucs-s-spar-viewer/qucs-s-spar-viewer.h
index ebfb67f7..1c801bb1 100644
--- a/qucs-s-spar-viewer/qucs-s-spar-viewer.h
+++ b/qucs-s-spar-viewer/qucs-s-spar-viewer.h
@@ -13,7 +13,9 @@
 #include <QtGlobal>
 #include <complex>
 
+#if QT_VERSION < QT_VERSION_CHECK(6,0,0)
 using namespace QtCharts;
+#endif
 
 class QComboBox;
 class QTableWidget;

@andresmmera
Copy link
Contributor Author

Thanks Vadim.

I'll apply that ASAP, probably this afternoon.

@dsm
Copy link
Collaborator

dsm commented Sep 9, 2024

I think this line should be this, #if QT_VERSION < QT_VERSION_CHECK(6,0,0)

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 9, 2024

I think this line should be this, #if QT_VERSION < QT_VERSION_CHECK(6,0,0)

Yes, the QtCharts namespace was deprecated since 6.0.0

@andresmmera
Copy link
Contributor Author

I have updated my Qt setup to v6.7.2. I've applied the patch above. Let's see what happens with the CI.

@dsm
Copy link
Collaborator

dsm commented Sep 9, 2024

You have to add qt6-charts:p under pacboy in ci for windows and move qt5charts installation line in appimage qt6 to cmake.yml

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 10, 2024

I have just checked Qt5.15 and Qt6.2.4 build on Ubuntu-22.04. Both Qt version setup compiles and works as expected. @andresmmera Please add the libqt5charts5-dev dependency in the .github/workflows/cmake.yml This will fix the Qt5 CI build.

@andresmmera
Copy link
Contributor Author

Sure. I'll do so this evening

@andresmmera
Copy link
Contributor Author

Done. Thank you @dsm and @ra3xdh

Remove unneeded CMakeLists in bitmaps folder


Add qucs-s-spar-viewer.icns

This is required for the "build-mac-universal" and "build-mac-intel" jobs
Install libqt5charts5-dev dependency

I set up a fresh Lubuntu 24.04 on a VM and compiled the code from source using the instructions provided in the Qucs-S page. I found that to avoid the above error in the build, you need to install "libqt5charts5-dev" first. 

After that the compilation went fine. I'm not sure if it works with "libqt6charts6-dev". I'll check later.
Typo in CMakeLists


Remove unneeded include causing trouble in build appImage-Qt6

/home/runner/work/qucs_s/qucs_s/qucs-s-spar-viewer/main.cpp:28:10: fatal error: QDesktopWidget: No such file or directory
   28 | #include <QDesktopWidget>


Missing line in windows build


Patch provided by ra3xdh QtCharts


Update cmake.yml and deploy.yml

Added qt6-charts to Windows build and qt5charts to Qt5 build
@andresmmera
Copy link
Contributor Author

It gave no errors.
Squashing the CI related commits.

@andresmmera
Copy link
Contributor Author

andresmmera commented Sep 10, 2024

I've found some warnings in the Qt6 build. I want to fix them before you merge this.

For some reason, I'm not catching them locally in QtCreator.

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 10, 2024

Is it ready for merge now?

@andresmmera
Copy link
Contributor Author

Yes. I think it's ready

@dsm
Copy link
Collaborator

dsm commented Sep 10, 2024

New tool is missing in macos ci, copy this tool to in qucs.app and run macdeployqt like other tools, there are two macos ci.

Copied in to build dir as qucs-sspar-viewer.app and run macdeployqt
@andresmmera
Copy link
Contributor Author

New tool is missing in macos ci, copy this tool to in qucs.app and run macdeployqt like other tools, there are two macos ci.

Done. Thank you!

@dsm
Copy link
Collaborator

dsm commented Sep 11, 2024

I'll try to test this tool on MacOS this evening

@andresmmera
Copy link
Contributor Author

@ra3xdh Don't merge this PR yet. I've found that the tool can load the same file twice.

I want to fix this issue before merging. I'll try to fix today.

This commit gets rid of QPalette. It doesn't work well on Windows. The color of the button is forced using the stylesheet.
@andresmmera
Copy link
Contributor Author

I've also added a commit to fix a wrong behaviour concerning the button styles in Windows.

@andresmmera
Copy link
Contributor Author

An aesthetic detail related to #929: I have noticed that Qucs-S shows the new icon but the tools keep an old one.

Should this be updated or left as it is?

photo_2024-09-13_09-32-20

@dsm
Copy link
Collaborator

dsm commented Sep 13, 2024

I think you can keep as it is, in macos, tools has different icon from qucs-s but that icon has only .icns file has not svg file so we should design a new icon for tools or use qucs-s.svg anywhere. @ra3xdh what do you think about it?

@ra3xdh
Copy link
Owner

ra3xdh commented Sep 13, 2024

@ra3xdh what do you think about it?

Let's keep the icons as is.

@ra3xdh ra3xdh merged commit 694db71 into ra3xdh:current Sep 15, 2024
7 checks passed
@ra3xdh
Copy link
Owner

ra3xdh commented Sep 15, 2024

I am merging this. I think the tool is mostly ready for usage. @andresmmera , Please stay connected if some feedback will appear from users after testing the next continuous release.

@andresmmera
Copy link
Contributor Author

Ok, no worries

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