SignalK / signalk-derived-data

Generates deltas for values derived from signalk values
Apache License 2.0
18 stars 30 forks source link

fix(moon): changed to type specific return values and added moon test #123

Closed godind closed 4 months ago

godind commented 9 months ago

Fixes #122

Previous implementation retuned all moon path values as strings. Adjusted return types to correct types and added tests.

godind commented 8 months ago

You are most probably right! To be honest I was just trying to fix the output to get cleaner types and did not look deeper. But I added some tests! It's just a quick patch.

Do you need more work done or is it good for now?

tkurki commented 8 months ago

Could you please fix the issues I pointed out.

godind commented 8 months ago

No problem. I'll look at it.

godind commented 8 months ago

I'll check this out. I dont understand why the date changes every time in tests since it provides static input coordinates and date time.

Could it be that while running the tests, if the server is playing stream file, that it overrides the static test data?

godind commented 8 months ago

I've fix the test issue, removed unnecessary variables, set default as undefined instead of ''. See if this is acceptable.

Thanks!

tkurki commented 8 months ago

There is no server involved in running the tests. I have no idea why the value would change, but the same tests still fail on my machine. Just remove the offending ones?

godind commented 8 months ago

It should work fine. Looks good on my end.

derived data converts
    ✓ Magnetic Course Over Ground[0] works
    ✓ Magnetic Variation[0] works
    ✓ Magnetic Variation[1] works
    ✓ Sets environment.moon.* information such as phase, rise, and set[0] works
    ✓ propulsion.port.slip (propulsion.port.transmission.gearRatio, propulsion.port.drive.propeller.pitch)[0] works
    ✓ propulsion.port.slip (propulsion.port.transmission.gearRatio, propulsion.port.drive.propeller.pitch)[1] works
  6 passing (10ms)
tkurki commented 7 months ago

It seems that the output varies for some reason, the tests fail on my machine

         {
           "path": "environment.moon.times.rise"
      -    "value": "2017-04-15T02:59:40.750Z"
      +    "value": "2017-04-16T03:52:46.109Z"
         }
         {
           "path": "environment.moon.times.set"
      -    "value": "2017-04-15T13:26:01.692Z"
      +    "value": "2017-04-15T13:25:54.314Z"
         }

Why don't you just remove the offending tests where the output varies for some reason? This PR does not change how the value is calculated, so this is a sidetrack.

Unless you want to really dig into what happens and fix things for all.

As long as the test fails I won't be merging this.

tkurki commented 7 months ago

The test is failing, because the output contains properties not in the expected block. You would have spotted that if you had ran the tests.

godind commented 7 months ago

I did not. I have a new machine and did not have complete setup so I just deleted the tests. That's what being out of time does... I'll get to it.

tkurki commented 6 months ago

Let's get this finished?

godind commented 4 months ago

Do I still need to fix something? I don't see it if so...

Thanks