Dc bias points bug

[nvdl]

#272

[guitorri]

Better use qDebug() << "message"; , so we can silence this on the released code.

[guitorri]

Your patch seems to fix the crash. Please squash to a single commit.

[guitorri]

For some reason, with your example, the spinbox does not update the net bias values... ?!

[in3otd]

...the values do not change because the sweep parameter R4 is not used...
@nvdl, the parameter R4 needs to be used as the value of a property, so if you would like to sweep the value of R4 you should assign R4 to its R property, i.e. have R=R4 in its properties.

[nvdl]

@in3otd I agree, realized that later.
But, did not consider that important to upload the edited one.
Can upload the edited example later if needed.

@guitorri You know now.
Better use qDebug() << "message"; , so we can silence this on the released code.
I did not write this but can update it.
Or better scan all the code for "cout" and replace that with "qDebug".

Current squash: 51ab6de

[in3otd]

@nvdl, no need to upload the edited example, just a comment when you spotted the unused sweep parameter would have been enough.

I see that 51ab6de contains the squashed commits but your branch dc-bias-points-bug still contains also the old commits plus a merge commit on top, which is strange.
I expected to see just a single commit after squashing all the commits.
I think you could do, on the dc-bias-points-bug branch
git rebase -i ced40f9
then leave just the line
pick 51ab6de D.C bias points calculation bug fix.
when selecting the commits to keep, and only the commit 51ab6de should remain.
Then use git push -f to update the branch on your work, to rewrite it with this single commit. This will be automatically reflected in this pull request.

[nvdl]

@in3otd
then leave just the line
pick 51ab6de D.C bias points calculation bug fix.
when selecting the commits to keep, and only the commit 51ab6de should remain.

I lost previous commits this way. The others need to be "squahed" or "fixed" in.
So it is not just one line that should be left in the "rebase" configuration.

Or possibly I did not follow you.

Nevertheless, I started from scratch and squashed commits.

[in3otd]

sorry @nvdl my understanding was that 51ab6de was already the result of the squashed commits. In any case your old commits should not have been completely lost, they are just not "linked" to the other commits anymore, Using git reflog you should be able to find their reference and access them. In the worst case, I still have a local copy of your branch, as it was when you opened the PR.

[nvdl]

@in3otd I did git reflog and squashed the commits again. Nothing lost on my side. Now, there is only one commit in the PR. Git is amazing once one figures out where the accelerator and brake is.

[in3otd]

Git is amazing once one figures out where the accelerator and brake is.

100% agree, the problem is when one mistakenly swaps the two; it happens, sometimes 😁

[guitorri]

Two things.

• @nvdl please try to keep commits independent of each other. You mixed up the fix for the issue with code formatting. This makes it hard to spot where the fix start/end. Two commits would be better in this case. Furthermore, we are planing to use a code formatting tool (clang-format) to format the whole codebase at once...
• after merging the code from @felix-salfelder the patch in this PR is not working anymore.

The crash is still happening when dereferencing the double pointers:
https://github.com/Qucs/qucs/blob/master/qucs/qucs/dialogs/sweepdialog.cpp#L49
https://github.com/Qucs/qucs/blob/master/qucs/qucs/dialogs/sweepdialog.cpp#L140
https://github.com/Qucs/qucs/blob/master/qucs/qucs/dialogs/sweepdialog.cpp#L194
https://github.com/Qucs/qucs/blob/master/qucs/qucs/dialogs/sweepdialog.cpp#L223

Need to sleep now...

[nvdl]

OK. I will have a look later.

[in3otd]

yeah, I was also taking a look at this yesterday evening, then almost fell asleep on the keyboard. My impression was that the problem might be in Graph::loadDatFile but I am really not sure. It might explain why the problem was also present in 0.0.18 .

I agree for the commit formatting, it was quite difficult to understand where the functional modifications and the "cosmetic" ones were. My suggestion is to avoid altogether to change the code formatting around: as guitorri said, we will (hopefully soon) have an automatic code formatting, as we know that the current style is not consistent.
Also I will suggest to use more descriptive commit messages. There are many guides around best practices for commit messages, see here and here. Also this is rather subjective, but the underlying principle is that code is read much more often than it is written. Imagine your commit message will be read a few months or years later, maybe by someone not familiar with that code and try explain in detail what you did and why. Sure, sometimes we do not follow this ourselves, but strive to improve our coding practices and set a higher standard. 😁
I suggest to update your master to pull in the latest changes and restart anew your branch from there.

[felix-salfelder]

guitorri: yes, i've run into the dc_bias code when experimenting with the data backend... i do not understand either the dc bias code nor the proposed PR -- now waiting for the cleaned up commit that in3otd is pointing to (i wouldn't mind to do the rebase to the current master).

[guitorri]

I will close this. Let us go back to the issue #272.

[nvdl]

Sure, looks like it needs a deeper dive.
Ofcourse, just checking for NULL pointers was hackish as I did not want to go to the origin of the function calls before the data was available.