maritime-labs / calypso-anemometer

Python driver for the Calypso Instruments Ultrasonic Portable Solar wind meter (UP10)
GNU Affero General Public License v3.0
12 stars 2 forks source link

Provide values in correct format for SignalK telemetry #14

Closed honigwald closed 1 year ago

honigwald commented 1 year ago

Dear @amotl, I'm glad that you did all the prework. I was able to use this project to connect my Calypso to my Signalk-Server / Openplotter-project. The problem was, that the angle was not stable and returning weird results in the windmeter instrument of signalk. This is because it awaits the AWA (apparent wind angle) in RAD and not in GRAD.

I changed in model.py and now everything is working fine. I suggest to change that in the repo for other people who want to use that.

BR Simon

https://github.com/maritime-labs/calypso-anemometer/blob/1cca7adc2440a2de7407bf620cc367efa1ecd74c/calypso_anemometer/model.py#L142

amotl commented 1 year ago

Dear Simon,

thank you for writing in.

I was able to use this project to connect my Calypso to my Signalk-Server / Openplotter-project.

Excellent!

The problem was [...]. I changed in model.py and now everything is working fine.

Can you tell me what exactly has been changed?

I suggest to change that in the repo for other people who want to use that.

I will be happy to consider it on the next iteration. Thank you already!

With kind regards, Andreas.

honigwald commented 1 year ago

If you like, I can try to create a PR that you can see my changes. I still was tinkering around, but the current result is sufficient enough

amotl commented 1 year ago

Sure, patches are always welcome! Thank you for your efforts already.

amotl commented 1 year ago

Hi again,

@UserMacUseface just reported a similar observation over at https://github.com/maritime-labs/calypso-anemometer/issues/12#issuecomment-1443668406 when using the SignalK telemetry adapter. It looks like SignalK actually expects all angle values to be in Radians. Is that an universal truth we may well have missed in our original implementation?

With kind regards, Andreas.

amotl commented 1 year ago

If my assumption is correct, the patch would need to go to the SignalK telemetry adapter file.

https://github.com/maritime-labs/calypso-anemometer/blob/1d38e093b92cccfcc8abbcde756e66d01770619f/calypso_anemometer/telemetry/signalk.py#L43-L55

While we discovered at https://github.com/maritime-labs/calypso-anemometer/issues/12#issuecomment-1443668406 that the compass heading angle also got tripped by this conversion flaw, I am also wondering about the other angle values pitch and roll?

amotl commented 1 year ago

It looks like SignalK actually expects all angle values to be in Radians. Is that an universal truth we may well have missed in our original implementation?

The documentation at ^1 clearly states all relevant angle values use rad (Radian) as unit. Apologies that we missed this. We will submit a corresponding fix and release a new version.

amotl commented 1 year ago

The code at ^1 clearly shows it is using the degreesToRadians() method to provide the correct unit to SignalK. We missed to follow this implementation correctly. Apologies.

const model = {
  windAngleApparent: this.degreesToRadians(windAngleApparent),
  windSpeedApparent: buf.readUInt16LE(0) / 100,
  batteryLevel: parseFloat(((buf.readUInt8(4) * 10) / 100).toFixed(2)),
  temperature: this.celciusToKelvin(buf.readUInt8(5) - 100),
  roll: this.compass === 0 ? null : this.degreesToRadians(buf.readUInt8(6) - 90),
  pitch: this.compass === 0 ? null : this.degreesToRadians(buf.readUInt8(7) - 90),
  compass: this.compass === 0 ? null : this.degreesToRadians(360 - buf.readUInt16LE(8)),
  timestamp: new Date().toISOString()
}

https://github.com/maritime-labs/calypso-anemometer/blob/1cca7adc2440a2de7407bf620cc367efa1ecd74c/calypso_anemometer/model.py#L139-L148

amotl commented 1 year ago

Observations

Other than the missed degreesToRadians() conversion, there is also:

Analysis

amotl commented 1 year ago

GH-20 will bring in corresponding fixes for the SignalK telemetry adapter. Thank you again for reporting this! We will add a few other improvements before running a new release.

honigwald commented 1 year ago

Hi @amotl I see you had some time today to dive into the code. Thanks a lot for your awesome work. The fix I did in my environment was more or less a bit hacky, so I'm happy and looking forward for the new release :)

Cheers, Simon.

amotl commented 1 year ago

Dear Simon,

we just released calypso-anemometer 0.6.0, including the corresponding improvements, and will be happy to hear back from you about its outcome on this matter.

When using the SignalK telemetry variant, other than correcting the angle values to be emitted in Radian (rad), the program will now also emit the temperature in Kelvin, and the battery level as a ratio/percentage, like signalk-calypso-ultrasonic.

With kind regards, Andreas.

honigwald commented 1 year ago

Thanks a lot Andreas for your great work! I did a quick test with the new vesion 0.6.0 and can confirm, that the values for AWA are provided in the correct format (RAD) now. Nevertheless the eCompass (Yaw/Heading, Pitch, Roll) and the Temperature don't provide any readings. image

amotl commented 1 year ago

Hi Simon,

excellent - thank you for reporting back. Let's close this issue then, and discuss the other malfunctions at GH-27?

Cheers, Andreas.