iobroker-community-adapters / ioBroker.fronius

ioBroker Adapter für Fronius Wechselrichter mit Fronius Datalogger Web ab Version 2.0.4-1 oder Fronius Datamanager ab Version 3.0.3-1
MIT License
22 stars 21 forks source link

[QA] eslint reports error at solarApiJson.js #300

Closed mcm1957 closed 1 year ago

mcm1957 commented 1 year ago

eslint .

/home/runner/work/ioBroker.fronius/ioBroker.fronius/test/solarApiJson.js 3:20 error Parsing error: Unexpected token =

✖ 1 problem (1 error, 0 warnings)


As an intermediate solution this file has been excluded from linting

mcm1957 commented 1 year ago

In addition I would suggest to check whther this file could be migrated to a pure data file. Currently it contains a class which exports several json strings referenced by variable names. It should be easy to migrate this to a pure json file read using require('filename')

nkleber78 commented 1 year ago

@mcm1957 to be honest i have no clue why eslint has a problem there. Do we have more information why the assignment to a variable is not accepted? If we migrate to a file, this would mean a file for each line / object correct? In General the question is if this test could be properly implemented as unit test instead of a manual test by checking the object creation. There i do not have experience with :(

mcm1957 commented 1 year ago

Sorry, I'm not really familar with classes :-) Maybe eslint simply misses thatthis file is a class file ? We could ask this at telegram developers channel.

Witrg reagrds to file: No, one single json fiule shoudl be sufficient - if I am correct.

If you convert the cs to a plain json file, i.e. as listed below:

solarApi.json:

{
    "testApiVersion" : {
        "APIVersion" : 1,
        "BaseURL" : "/solar_api/v1/",
        "CompatibilityRange" : "1.5-9"
    },
    "testInverterRealtimeDataCommon" : {
        "Body" : {
            "Data": {
                "DAY_ENERGY" : {
                    "Unit" : "Wh",
                    "Value" : 1393.2
                    },
                "DeviceStatus" : {
                    "ErrorCode" : 0,
                    "LEDColor" : 2, 
                    "LEDState" : 0,
                    "MgmtTimerRemainingTime" : -1,
                    "StateToReset" : false,
                    "StatusCode" : 7
                    },
                "FAC" : {
                    "Unit" : "Hz",
                    "Value" : 49.97
                    },
                "IAC" : {
                    "Unit" : "A",
                    "Value" : 0.36
                    },
                "IDC" : {
                    "Unit" : "A",
                    "Value" : 0.32
                    },
                "PAC" : {
                    "Unit" : "W",
                    "Value" : 84
                    },
                "TOTAL_ENERGY" : {
                    "Unit" : "Wh",
                    "Value" : 1734796.12
                    },
                "UAC" : {
                    "Unit" : "V",
                    "Value" : 232.4
                    },
                "UDC" : {
                    "Unit" : "V",
                    "Value" : 399.9 
                    },
                "YEAR_ENERGY" : {
                    "Unit" : "Wh",
                    "Value" : 322593.5
                    }
                }
            },
        "Head" : {
            "RequestArguments" : {
                "DataCollection" : "CommonInverterData",
                "DeviceClass" : "Inverter",
                "DeviceId" : "1",
                "Scope" : "Device"
                },
            "Status" : {
                "Code" : 0,
                "Reason" : "",
                "UserMessage" : ""
                },
            "Timestamp" : "2019-06-12T15:31:03+02:00"
            }
        }
    }

you shoudl be able to read it using

const testdata = require('solarApiJosn.json');

and access the json data using i.e.

testdata[testInverterRealtimeDataCommon] like with any other json structure,

So with optimal naming it looks like that no code change would be required.

But please test before convertsing the complete file :-)

And yes - this is NOT prior.

mcm1957 commented 1 year ago

I just fixed this issue.

plain variables at classes seems to be ruther new feature. So the eslint parser must be set to 2022 mode (was 2020 mode) See https://stackoverflow.com/questions/60046847/eslint-does-not-allow-static-class-properties

I've fixed the other eslint issues like unneede escapes too.

This issue can be closed. No need to spent time to migrate to json file - unless you are interested for knowledge.

nkleber78 commented 1 year ago

@mcm1957 i just did the move to the file. It is working. I also fixed the eslint issues with the hasOwnProperty as well

mcm1957 commented 1 year ago

@nkleber78 OK, thanks a lot. Thats (in my opinion) the cleanest solution - especially in reard of future maintanance.

nkleber78 commented 1 year ago

But unfortunately this cases issues for the build as the folder is "test". Is there a way to make this import and the rest of the code only availlable when needed or ignore the "module not found" in case the module is not in test

mcm1957 commented 1 year ago

Sorry for the late reply.

What kind of problems do you have?

I think one or more of the following should work:

-) you can move the solarApiJosn.json to lib directory. Its does not harm. And it would open the possibility to add a flag "testmode" to config to run the testmode at user systems too.

-) I do not see a reason why keeping the require inside the if statement just like curent code should not wiork. So the require will be executed only if configured in source

-) I assume require raises an exception if the file does notz exist. This could be cateched and ignored using try{} catch(e){} construct.

Please let me know if I can help more - feel free to ask at telegram as I will note this faster

nkleber78 commented 1 year ago

can you help me by adding the testmode flag to the config. I think this is a good idea. I moved the JSON to the lib directory. I would propose to do a merge of the PR and adding the testmode to the config

mcm1957 commented 1 year ago

For a first test please simply use a boolean variable (i.e. testMode) in main.js. You can later set it by reading config.xxx. (But as far as I know there was some sort of such a variable anyway. But I did not scan last code)

I will add the test flag when providing jsonConfig as I cannot write (good) html / react code.

nkleber78 commented 1 year ago

Perfect, it is already a variable in the code so we can use that later in the jsonConfig

mcm1957 commented 1 year ago

fixed with 2.0.0-alpha.4