Fails to calculate true wind with some NMEA message combinations

[ARF1]

Great plugin! I have always been wondering: "Where will I end up if I tack now? ... Is it still too early to make it arount the cape."

From what I understand, this plugin is supposed to calculate true wind if apparent wind and magnetic heading are provided on NMEA. For some combinations of NMEA sentences this does not seem to work.

If I send the following (artificial, because I am at home at the desk) sentences, true wind is not calculated:

$IIMTW,30.10,C*21
$IIMWV,64.00,R,20.70,K,A*3F
$IIVHW,,T,179.50,M,30.00,N,5,K*59
$IIHDM,179.50,M*18
$GPGLL,3553.5295,N,13938.6570,E,002454,A,A*4F
$GPGGA,002454,3553.5295,N,13938.6570,E,1,05,2.2,18.3,M,39.0,M,,*7F
$GPVTG,160,T,034.4,M,005.5,N,030,K*49

I have the WMM installed and activated.

Theoretically, with the magnetic heading (in HDM and VHW), the apparent wind (in MWV) and the declination (from WMM plugin) true wind calculation should happen automatically, right?

Instead only apparent wind data is recognized. If I "force calculation of true wind" in the settings, it works. But from my understanding of the docs, this setting is only intended to replace any existing true wind data on the NMEA stream. It should not be require to supplement an incomplete NMEA stream, right?

So is this a bug I did I misunderstand the documentation?

[rgleason]

If you are doing this, you'd better have a full explanation why and what it is for, either in a pop message at the option selectors, or in the menu. It should be absolutely clear what you are doing. Also if this is accepted, please provide a complete description for the manual, when it is to be used, why it is necessary, and any possible issues.

PS: Thoroughness of Your documentation and popup or menu explanation is likely to determine if the PI authors accept it.

[pa2wlt]

@ARF1 I fully agree: the behavior should be well documented in the manual, that ain't the hardest part I would say. Regarding the treshold value: the value itself is quite easy to change in the code, since it is just a one time number. We can start with a certain value, see how that goes, experiment. Would adding an extra user-settable option for this make sense? I personally wouldn't need it and can't think of a good reason that would justify again another parameter. Perhaps someone else can. I reckon a fixed value would be good enough to start. Agree with @rgleason as well. If we would add such an option we have to explain it very very well.

Regarding the PI authors: I haven't heard anything from Thomas yet. Not on the comments, not on the PullRequest. The code has been done and tested, and works great. Just have to wait for him to respond on the PullRequest.

Petri did respond immediately. He had issues with the wind calculation, whereas it is not completely clear to me what issue he solved. He is too busy and lacks the time to answer questions. Fact is that the windcalculation in his code now has a way higher cyclomatic compelexity, also because of all debug statements. Changing that will cost some time. From a professional perspective this particular code should be on top of the list to refactor though, since it is impossible to test this costefficiently.

Besides that, I am not sure how the codebase will evolve if Petri isn't spending time on it anymore. In that case it is a waiste of time to spend my energy on it. Then I'd rather redo the complete plugin, in small, simple units, with low complexity and automated test code. That will cost even more time, but is a more future proof solution. However, then there will be 4 plugins regarding dashboards and tactics ... There should be a better way to get the improved True Wind Calculation available.

The 3 step I will explore is to change dashboard_pi and add True Wind Calculation in there. I am aware of your doubts about that @rgleason, though I really need the calculations, and will continue until that's done. Changing that codebase shouldn't be too hard, since all the code required for the true wind calculation is available. The issue with this plugin however, is that I haven't found a way yet to build only dashboard_pi.dll, without building opencpn and the other plugins. I know how to code (and write maintainable code), though I am not an expert on the cmakefiles yet. That'll take some extra time as well, though it might be less time consuming than redoing tactics. Tbc.

[rgleason]

If you want just dashboard, Petri's version has a switch, to include Tactics or not. You could fork his, build your version of wind (I think you have done that) and just build dashboard without tactics.

Petri's code is fine IMHO. Its gone through codacy has an "A" and is substantially better than before because he has taken some time on it. I doubt it is a good idea to make a lot of changes because then it will need to be tested again!

Some of the problems Petri found were from unexpected causes, so if you really want to polish the code up, you'll want to be very careful, and you'll also need to go though testing again, which is not a quick thing to do if you are doing it correctly.

[pa2wlt]

Thx again. I haven't found that switch yet. I forked the code bases already, found a way to build the dashboard_pi.dll and will see what it takes to change the functionality of Dashboard. I reckon quite some sailors will be happy if the standard module also calculates the true wind if TWS, TWA and either STW or SOG are available.

Regarding Petri's code: code quality is not a mater of opinion. One can make objective measurements and reason about facts. I.e. count the number of lines in a unit, calculate the cyclomatic complexity and then see how these scores relate to what is standard in industry. All based on ISO 25010 and its predecessors. The complete code base of OpenCPN, the Plugins and Petri's cosebase score low on maintainability. E.g. on long units, high cyclomatic complexity and redundancy. See also the forum topic and read there why Codacy is of no use here yet (sorry). I wish it was useful. On the longer term Codacy will certainly support complexity for C++ as well (they said) though not yet....

Nevertheless, I do agree very much with you that in Petris code regarding the True Wind Calculations one shouldn't make changes. That is way too tricky. Refactor it, with simple units with low complexity and write automated test code. That is the way forward. And yes, that is not a quick thing, since it requires a lot of functional testing as well. Prior to that it requires a lot of work already to figure out what has been done in the codebase (compared to Thomas'). And again: From a functional perspective I am happy Petri did combine both plugins and made the functional improvements. Thx!

I'll focus on Dashboard first, wait for Thomas' response regarding the Pull Request, and leave Petri finish his project. In the mean time I'll keep an eye on the forum to see if there is some enthusiasm regarding improving maintainability.

[pa2wlt]

Responding here takes more time than changing the code base. ;-) Just managed to change the code in dashboard_pi so that TrueWind is calculated. Nice. Less than an hour of work. I will also see if I can add the same menu as in Thomas' to set the option to force, use SOG or STW. Test is thoroughly as well, and ask Petri if there are other issues in Dashboard to solve. And afterwards do it decent with a fork, and pull request. That must lead to a change in OpenCPN someday.

[rgleason]

https://github.com/tom-r/tactics_pi/pull/45