Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Altitude is reported incorrectly via FANET protocol #64

Closed
1 of 6 tasks
lenalebt opened this issue Sep 2, 2019 · 15 comments
Closed
1 of 6 tasks

Altitude is reported incorrectly via FANET protocol #64

lenalebt opened this issue Sep 2, 2019 · 15 comments
Assignees
Labels

Comments

@lenalebt
Copy link

lenalebt commented Sep 2, 2019

First: I have seen #45 and that it was closed (inalid template usage), but I think they are related.

Hardware

  • Prime Mark II
  • Standalone
  • UAV
  • UAT module
  • SkyView
  • Flight Recorder

photo_2019-09-02_09-58-55

Firmware version

1.0-rc6 ESP32

photo_2019-09-02_09-59-21

Firmware settings

photo_2019-09-02_09-59-25

Describe the bug (or ask a question): BUG REPORT

GPS height reported by the device seems right, but height reported via FANET protocol seems to be wrong (off by about 780m). I used 2 devices (both Prime Mark 2, from different resellers), directly besides each other (see picture above). XCSoar shows the height correctly (using GPS from SoftRF, disabling internal GPS) as 811m (see screenshot:)
photo_2019-09-02_09-59-12

In FLARM radar however, the height of the other device is shown incorrectly (as -777m relative):

photo_2019-09-02_09-59-16

This view shows absolute heights:

photo_2019-09-02_10-06-58

I also had a colleague with a Skytraxx 3.0 check my reported heights, and they are reported as the same (around 15-40m) for him, from both devices. I can see his height correctly in XCSoar via SoftRF. (no photos here, if you need them I can take them later). I tried exchanging the devices (connecting to the other Prime Mark II), which did not change the situation.

See this NMEA log (captured via serial port from the computer):
capture.txt

You can see that the reported heights are indeed relative negative around 780m as in this line:
$PFLAA,0,-1,-5,-780,3,07A370!FAN_07A370,330,,0,0.0,7*4D

I am unsure how to capture GDL90, but if you need it I can provide that as well.

Since the heights are also reported incorrectly on a device that is not running SoftRF firmware, I think it must be somewhere on the sending side, not the receiving side, but this is a pure guess.

To Reproduce

Set up two devices besides each other, observe reported heights with FANET protocol via NMEA. The heights are reported incorrectly.

Expected behavior

The heights are reported correctly (up to GPS accuracy +-10-20m).

@lyusupov
Copy link
Owner

lyusupov commented Sep 2, 2019

Please, provide status screenshots and serial output capture from both SoftRF units (close to each other, good satellites reception and at the same altitude).

@lenalebt
Copy link
Author

lenalebt commented Sep 2, 2019

Okay, there you go:

device 1:
capture-25A370.txt
25a370
25a370-settings

device 2:
capture-C56E20.txt
c56e20
c56e20-settings

8 satellites reception, <1m near each other.

Here a picture of the skytraxx device seeing the 56e20 device, as well as it showing it's own altitude:

skytraxx

skytraxx2

@lyusupov
Copy link
Owner

lyusupov commented Sep 2, 2019

Thanks!

I am currently in travel and will not be able to validate your report on the hardware I have
until at least end of this week (or begin of next week) .
If you have enough skills to build the firmware from source code and flash the binary into your unit -
I would suggest you to alter line 322 in Protocol_FANET.cpp from:

pkt->altitude_msb   = (altitude & 0x700) >> 16;

onto

pkt->altitude_msb   = (altitude & 0x700) >> 8;

@lenalebt
Copy link
Author

lenalebt commented Sep 2, 2019 via email

@lyusupov lyusupov self-assigned this Sep 2, 2019
@lyusupov lyusupov added the bug label Sep 2, 2019
@lenalebt
Copy link
Author

lenalebt commented Sep 2, 2019

Tried it, but cannot get it to link properly. This is the error message I get:

/home/lena/.arduino15/packages/esp32/hardware/esp32/1.0.1/tools/sdk/lib/libbt.a(bt.o):(.literal.esp_bt_controller_enable+0xc): undefined reference to `btdm_rf_bb_init'
/home/lena/.arduino15/packages/esp32/hardware/esp32/1.0.1/tools/sdk/lib/libbt.a(bt.o): In function `esp_bt_controller_enable':
/local_disk/lyusupov/tmp/esp-idf/components/bt/bt.c:940: undefined reference to `btdm_rf_bb_init'
/home/lena/.arduino15/packages/esp32/hardware/esp32/1.0.1/tools/sdk/lib/libbt.a(bta_jv_act.o):(.literal.bta_jv_start_discovery_cback+0x8): undefined reference to `lwip_ntohs'
/home/lena/.arduino15/packages/esp32/hardware/esp32/1.0.1/tools/sdk/lib/libbt.a(bta_jv_act.o):(.literal.bta_jv_start_discovery_cback+0xc): undefined reference to `lwip_ntohl'
/home/lena/.arduino15/packages/esp32/hardware/esp32/1.0.1/tools/sdk/lib/libbt.a(bta_jv_act.o): In function `shorten_sdp_uuid':
/local_disk/lyusupov/tmp/esp-idf/components/bt/bluedroid/bta/jv/bta_jv_act.c:811: undefined reference to `lwip_ntohs'
/local_disk/lyusupov/tmp/esp-idf/components/bt/bluedroid/bta/jv/bta_jv_act.c:817: undefined reference to `lwip_ntohl'
libraries/BTClassicSPP/BTSPP.cpp.o:(.literal._ZL13esp_bt_gap_cb21esp_bt_gap_cb_event_tP21esp_bt_gap_cb_param_t+0x10): undefined reference to `esp_bt_gap_pin_reply'
libraries/BTClassicSPP/BTSPP.cpp.o: In function `esp_bt_gap_cb(esp_bt_gap_cb_event_t, esp_bt_gap_cb_param_t*)':
/home/lena/Arduino/libraries/BTClassicSPP/BTSPP.cpp:688: undefined reference to `esp_bt_gap_pin_reply'
collect2: error: ld returned 1 exit status

I used the libbt.a from the repo, as described in the README. I tried both versions 1.0.1 and 1.0.2 of the ESP32 SDK.

@lyusupov
Copy link
Owner

lyusupov commented Sep 2, 2019

INSTRUCTIONS

  • 1.0.2 is NOT recommended
/local_disk/lyusupov/tmp/esp-idf/components/bt/bt.c:940: undefined reference to..
  • libbt.a from this location is NOT applicable for 1.0.1 Use the binary that comes with 1.0.1

@lenalebt
Copy link
Author

lenalebt commented Sep 2, 2019

Okay, that fixed it. Thank you :)! I just uploaded the suggested change to one of the devices, and it worked. I will create a PR for the change and attach it here.

lenalebt added a commit to lenalebt/SoftRF that referenced this issue Sep 2, 2019
Height has been incorrectly reported via FANET protocol. Suggested change came from lyusupov.
Co-authored-by: lyusupov
@lyusupov
Copy link
Owner

lyusupov commented Sep 2, 2019

If this was sufficient - there is no need for a PR.

Be aware that your C56E20 unit is having non-genuine NEO GPS module been soldered on the board.
It is not a critical issue. Think about to uncomment line 219 in your copy of Platform_ESP32.h:

//#define USE_AT6558_SETUP

@lenalebt
Copy link
Author

lenalebt commented Sep 2, 2019

It was sufficient - PR was already there. You can merge it or decline, both fine for me as long as the fix goes into the product :).

Thanks for the hint - I already was curious why the output was different for both devices. Fun fact: The device I ordered directly from china is the one with the genuine chip, the other one (bought on the airwhere homepage) had the non-genuine GPS chip. With USE_AT6558_SETUP, output of both devices via serial looks the same.

@lyusupov
Copy link
Owner

lyusupov commented Sep 3, 2019

  1. "height" (in the issue's subject ) is not an aviation term. This word is in use as one of an object's dimension characteristics , like L x W x H of an aircraft.
    Aviation terms are "altitude" and "elevation".

  2. my patch does not qualify yet as a fix for incorrect altitude reporting. Not every case has been tested yet. I am going to process it when I will return from my tour.
    your commit does not qualify as a contribution because there were no any of your own R&D behind that. I approve a PR when a contributor has done full R&D by him- or her- self entirely, without my intervention. Hence, when he/she saved my own time. For example of a good, approved PR, see this one

  3. LilyGO company had issues in Fall of 2018 when they did inappropriate sourcing of GPS modules for every of T-Beam production batches.
    Later on, LilyGO representative told me that they had solved this issue:

We have not used the previous module since we last talked about this problem.
Now we are getting the goods directly with the original UBLOX agent.

     I saw no reports on 'fake NEO' issue with T-Beams since then.
     I suppose that your 'fake NEO' T-Beam belongs to one of that old batches.

lenalebt added a commit to lenalebt/SoftRF that referenced this issue Sep 4, 2019
Altitude has been incorrectly reported via FANET protocol. Suggested change came from lyusupov.
Co-authored-by: lyusupov
@lenalebt lenalebt changed the title Height is reported incorrectly via FANET protocol Altitude is reported incorrectly via FANET protocol Sep 4, 2019
@lenalebt
Copy link
Author

lenalebt commented Sep 4, 2019

Your project - your rules ;).

I also changed altitude calulation above 2047m (confirmed wrong yesterday). I'd add unit tests (e.g. property-based tests to verify that fanet_decode(fanet_encode(x)) == x (circa) for all sensible x), but could not find a spot to add some.

Same here - you may use it, or discard it. In the end I'm not interested in the "fame", I just want it fixed :).

@lyusupov
Copy link
Owner

lyusupov commented Sep 6, 2019

The issue has been reproduced.

Resolution: confirmed

@lyusupov
Copy link
Owner

lyusupov commented Sep 6, 2019

Fix has been issued in this commit.

@lenalebt
Copy link
Author

lenalebt commented Sep 8, 2019

Confirm that the commit fixes the issue, both for altitudes <2047m and >=2047m.

@lyusupov
Copy link
Owner

lyusupov commented Sep 8, 2019

Thank you for the confirmation!

If the issue is no longer pertinent for you - please, close it.

@lenalebt lenalebt closed this as completed Sep 8, 2019
Repository owner locked as resolved and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants