TrendGroup does not set Type::FROM when parsing "FM"

[bovine]

Opening as a separate issue... Here's another case I found:

YBBN 132300Z 22007KT 9999 FEW020 26/21 Q1006 FM0200 12008KT 9999 SCT030

That gets called with:

CloudGroup FEW020 TrendGroup Type::NONE, FM0200 CloudGroup SCT030

I think the TrendGroup should have been called with Type::FROM instead? Do you agree?

Perhaps this block needs to set result.t = Type::FROM; https://github.com/nnaumenko/metaf/blob/59267179360103ba1822883fab059665c167c91f/include/metaf.hpp#L4098

Originally posted by @bovine in https://github.com/nnaumenko/metaf/issues/1#issuecomment-586046077

Here's a condensed program for the above test case:

#include <stdio.h>
#include "metaf.hpp"

using namespace std;
using namespace metaf;

int main(int argc, char *argv[])
{
    class MyVisitor : public Visitor<int> {
    public:

    private:
        virtual int visitFixedGroup(const FixedGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitLocationGroup(const LocationGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitReportTimeGroup(const ReportTimeGroup & group, ReportPart reportPart, const string & rawString) { return 0; }

        virtual int visitTrendGroup(const TrendGroup & group, ReportPart reportPart, const string & rawString) {
            string typeStr;
            switch (group.type()) {
            case TrendGroup::Type::NONE: typeStr = "Type::NONE"; break;
            case TrendGroup::Type::NOSIG: typeStr = "Type::NOSIG"; break;
            case TrendGroup::Type::BECMG: typeStr = "Type::BECMG"; break;
            case TrendGroup::Type::TEMPO: typeStr = "Type::TEMPO"; break;
            case TrendGroup::Type::INTER: typeStr = "Type::INTER"; break;
            case TrendGroup::Type::FROM: typeStr = "Type::FROM"; break;
            case TrendGroup::Type::TIME_SPAN: typeStr = "Type::TIME_SPAN"; break;
            default: typeStr = "Unknown"; break;
            }
            printf("TrendGroup %s %s\n", typeStr.c_str(), rawString.c_str());
            return 0;
        }

        virtual int visitWindGroup(const WindGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitVisibilityGroup(const VisibilityGroup & group, ReportPart reportPart, const string & rawString) { return 0; }

        virtual int visitCloudGroup(const CloudGroup & group, ReportPart reportPart, const string & rawString) {
            printf("CloudGroup %s\n", rawString.c_str());
            return 0;
        }

        virtual int visitWeatherGroup(const WeatherGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitTemperatureGroup(const TemperatureGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitTemperatureForecastGroup(const TemperatureForecastGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitPressureGroup(const PressureGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitRunwayVisualRangeGroup(const RunwayVisualRangeGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitRunwayStateGroup(const RunwayStateGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitSecondaryLocationGroup(const SecondaryLocationGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitRainfallGroup(const RainfallGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitSeaSurfaceGroup(const SeaSurfaceGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitColourCodeGroup(const ColourCodeGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitMinMaxTemperatureGroup(const MinMaxTemperatureGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitPrecipitationGroup(const PrecipitationGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitLayerForecastGroup(const LayerForecastGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitPressureTendencyGroup(const PressureTendencyGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitCloudTypesGroup(const CloudTypesGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitCloudLayersGroup(const CloudLayersGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitLightningGroup(const LightningGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitVicinityGroup(const VicinityGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitMiscGroup(const MiscGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
        virtual int visitUnknownGroup(const UnknownGroup & group, ReportPart reportPart, const string & rawString) { return 0; }
    };

    ParseResult parsed;
    parsed = Parser::parse("YBBN 132300Z 22007KT 9999 FEW020 26/21 Q1006 FM0200 12008KT 9999 SCT030");

    MyVisitor visitor;
    for (const auto groupInfo : parsed.groups) {
        visitor.visit(groupInfo);
    }

    return 0;
}

Originally posted by @bovine in https://github.com/nnaumenko/metaf/issues/1#issuecomment-586388155

[nnaumenko]

Thank you for finding this. I have also checked the other METARs from Australia and found this:

METAR YBBN 041130Z 15010KT 9999 SCT013 BKN028 BKN045 22/19 Q1018 FM1130 16011KT 9999 -DZ FEW010 BKN015 INTER 1130/1300 16020G30KT 3000 SHRA BKN010 TEMPO 1300/1430 2000 DZ BKN010 FM1130 MOD TURB BLW 5000FT TL1200=

Currently FMxxxx or TLxxxx or ATxxxx are expected to follow TEMPO or BECMG or INTER, and they are assigned Type::TEMPO or Type::BECMG or Type::INTER accordingly. Otherwise FMxxxx or TLxxxx or ATxxxx are assigned Type::NONE (when designing TrendGroup I assumed it does not happen in real life).

I agree on assigning Type::FROM to lonely FMxxxx group.

Regarding the rest, I think I’ll remove Type::NONE and add Type::UNTIL and Type::AT for lonely TLxxxx and ATxxxx groups.

Sequence FMxxxx TLxxxx not preceded by TEMPO or BECMG or INTER will be assigned Type::TIME_SPAN.

Also based on the METAR above, at the moment trends like TEMPO 1300/1430 or INTER 1130/1300 are not recognised, I would add them as well.

I can add all of this to version 5.0.0 and roll it out this week, what do you think?

[bovine]

ok sure, that release schedule is fine.

I may be able to identify more issues soon, as I am continuing to do more comparison/validity work this week.

[nnaumenko]

Added to the upcoming version 5.0.0 in commit 2735121.