betaflight / betaflight

Open Source Flight Controller Firmware
GNU General Public License v3.0
8.6k stars 3.03k forks source link

[Bug?] Gps alt not used #4800

Closed wx4cb closed 6 years ago

wx4cb commented 6 years ago

Dont have a baro attached to my plane but i am using a gps. I searched thru rc groups and google but couldnt find anything. The gps alt shows ok in the configurator but the osd shows 0 for altitude even though gps altitude is selected

Any way to fix it?

jflyper commented 6 years ago

GPS altitude item must be added.

wx4cb commented 6 years ago

Its added in the osd if thats what r asking? When i get home, ill take a look the osd code and see if i can do it

McGiverGim commented 6 years ago

No, he says that it must be added to the code.

jflyper commented 6 years ago

Yeah, there is no "GPS altitude" display item.

wx4cb commented 6 years ago

ok that's what I thought he meant. well a suggestion (and i just got home so i'll take a look) i think the easiest way to do it would be instead of moddingt he osd code to add that is to say something along the lines of:

ok there's no baro and my "altitude" is zero, but my gps altitude is xxx - copy gps altitude into regular altitude variable.

That way no osd code needs to be modified and that should be an easier mod to do.

McGiverGim commented 6 years ago

@wx4cb I think that you send the PR to cleanflight and not betaflight...

wx4cb commented 6 years ago

@McGiverGim correct i don't know how to delete it from there and put it in the betaflight one. :P

McGiverGim commented 6 years ago

You have a "Close" button at the end of the PR. Close it. And open a new PR against betaflight.

wx4cb commented 6 years ago

@McGiverGim ok id that, but it's asking me to compare branches when i go into pull requests in betaflights github, it changes it to cleanflights github

McGiverGim commented 6 years ago

But do you have the code with the change done? If you have the code with the change, then is you are doing a PR, if not, you're wrong and you are asking for a feature request, that is an issue (like this) explaining what you want.

wx4cb commented 6 years ago

@McGiverGim I was trying to do it myself to help you guys out as I know you guys are busy. but i think it may be > than my coding skills lol I think im just going to leave it as a FR right now :dagger: But from what i've seen it might be just a simple change to:

flight/altitude.c to set "estimatedAltitude" to the GPS altitude

AirBreak69 commented 6 years ago

@McGiverGim @jflyper Since some months i have a private version of osd.c and gps.c that display some GPS information on OSD, if no mag and baro is available. Unfortunately i couldn't get over the GIT hurdles to publish them. Probably one of you could do this for me - a great chance for me to get it public...

gps_M8M_Galileo_20171220.zip osd_GPSaltDist_2Home_20171220.zip

Some additional info: Title: GPS altitude/course instead of baro/mag in OSD, w/ GPS w/o baro/mag

This PR is for those who like to have altitude/course via GPS information in OSD without having baro and mag.

Features (osd.c):

Features (gps.c):

To be reviewed:

wx4cb commented 6 years ago

@AirBreak69 that's interesting - i'll have to take a look at that

McGiverGim commented 6 years ago

@AirBreak69 I tried to use the GPS Hold and RTH functions in a switch (for some emergency situations) and I finished with the same problems as you, unusable. I tried to find the problem without luck, so maybe we will need to wait until someone with more knowledge modifies the code.

Very interesting your features, I can make the PR, if you want, but I'm pretty sure that changes/revisions will be asked, so is easier that you own the repository and you make the changes to the code. We will be happy to help you with git, here or in the slack chat.

AirBreak69 commented 6 years ago

@McGiverGim Thanks for your offer. It took me several nights in reading a lot of docs. I tried the cygwin and the windows flavor. But it allways ended in very frustrating "access denied" errors.

The changes aren't really invasive and simply maps raw GPS data to OSD's altitude and direction. But i think, one of the guys who have the "bigger picture" should review the code.

McGiverGim commented 6 years ago

I know the vast majority of users will told you that is better to use the command line for git... But I use Eclipse who has git integrated and it's easier 😂

jflyper commented 6 years ago

@AirBreak69 The access denied error is probably caused by trying to directly push (or publish) changes to the GitHub repository that you don't have write access to.

mikeller commented 6 years ago

One thing to add to @jflyper: Don't be shy to ask if you can't get something to work - we're always happy to help users who want to contribute to Betaflight. For direct chat, you can also join the Betaflight Slack (registration on http://betaflight.ch/).

jflyper commented 6 years ago

Yeah, I'm asking (rather stupid) questions all the time 😬 @mikeller knows...

AirBreak69 commented 6 years ago

@jflyper @mikeller Thanks a lot for your hints. The main problem was that i created a fork without having a personal fork. After creating it using the web interface, i could manage it somehow (but will never be able to do it again ;-)).

4817 carries my little contribution.

beeb commented 6 years ago

Nice! I suggested such modifications for home arrow in issue #4430, good to see it's becoming a reality!

AirBreak69 commented 6 years ago

Thanks! @mikeller motivated me to think about other routines to implement it. So I reworked my changes totally. Unfortanetly GIT is fooling me since some weeks, I'am not able to publish it. For anyone who wants to try it, here's a ZIP containing all the changed file based on build 1928 and a new description. Airbreak69_Releases_20170101_based_on_build1928.zip

mikeller commented 6 years ago

@AirBreak69: Where are you stuck with in git? You should be able to just push your latest changes into the branch with the existing pull request, and the pull request will update automatically. If you rebased your changes or otherwise rewrote the branch history, a git push --force-with-lease will be needed.

peteoz commented 6 years ago

hey guys, this is some exciting development in progress. thanks @AirBreak69 & @beeb for suggesting the idea.

are there already any compiled versions (.hex) to test ? (I'm running CLRacing F4S).

current BF implementation is better than nothing but the home arrow based on accelometer is not working great for me. it starts of well but then it drifts over time (my virtual compass is drifting by about 90deg anti-clockwise every 2 minutes).

using approximate direction (in the absence of compass) from last known GPS points is the way to go in my opinion and I understand this would not work very well for acro flying in the park, but for us who are into long range flights (going out generally in one direction) this would be excellent alternative on the boards that don't support I2C or when we want to use simple micro Ublox units with no compass.

cheers

AirBreak69 commented 6 years ago

@peteoz: Please give a short comment if you were able to test my custom compiled version.

peteoz commented 6 years ago

@AirBreak69 I downloaded the ZIP file above but there is no compiled version, only .c .h project files, where can I find compiled version ? I would love to test this.

AirBreak69 commented 6 years ago

I send you an email containing everything and did not get 'not delivered' error. Did it reach you?

Abancalari commented 6 years ago

@AirBreak69 If you post the hex files somewhere, I'm sure other people would also be willing to test this out (myself included) =)

peteoz commented 6 years ago

@AirBreak69 sorry, I didn't get the email. even checked my spam box now and it isn't there either.

AirBreak69 commented 6 years ago

I think that posting hex files here is not a good style. Interested users should try to build their own custom builds based on the downloadable repositories. The lastest adaptation for enthusiasts: Airbreak69_Releases_20170109_based_on_build1942.zip

But currently I have some problems in publishing my code changes. I'll do a temporary exception and provide one special file here (will be reverted) to receive some feedback ... @peteoz I wasn't aware that github silently absorbes mails to mail notifications owners. Here is a ZIP containing 2 hex files for your FC made from different snapshots of the general code base. Please read the attached text file to get the feature activated. I hope you know how to save and restore your FC settings - they may get lost after flashing. Note: I don't think that the current code base is deeply flight tested. Airbreak69_PreRelease_for_CLRacingF4_20170109_based_on_builds_1934_1942.zip

peteoz commented 6 years ago

@AirBreak69 thank you very much for this, I understand this was not fully tested yet but I'll give it a go. in CLI I'm going to set "force_gps_heading_altitude=ON"

wx4cb commented 6 years ago

@AirBreak69 if it's code thaty ou're familiar with and are working on, it's definately advantageous to post a hex here imho. it saves people here who may not be that ofay with reading the code and the changes to know what's going on exactly, whereas you can have plenty of people testing it. it's not like it's in the repository etc yet :D

mikeller commented 6 years ago

@AirBreak69: If you do not want to post hex files for testing (I think there is nothing wrong with that practice), then it would be a lot more practical if you simply posted a link to the branch of your fork that contains the code changes, instead of zipping up source files. This makes it a lot easier for users to compare your changes with other branches of the source. Zipping up sources and posting them to GitHub does not make much sense - after all it's GitHub. ;-)

peteoz commented 6 years ago

@AirBreak69 I was able to test the build "BF_3.3.0b1942" today. The weather wasn't great so I only flew 5 batteries. Both home arrow and the altitude were working quite well. I have intentionally positioned the quad facing South (instead of North), after take off the home arrow is off for little while, sometimes it takes 15 seconds and worst I had was 60 seconds before it starts pointing correctly, but once it "catches" correct direction then it's 100% spot on for the rest of the flight. It even reacts very quickly to changes of direction.

I was flying from a ridge above the ocean so it was fairly easy to check how the altitude metering works. It's also fairly accurate (I would say on average +/- 5m). My launch location was about 100m above the sea level and as I got down to the beach it went close to 0. One thing I noticed - when it goes below 0m (because the accuracy is not 100%) then sometimes it shows negative numbers (ie. -0.7m) but on some flights it was showing 65535 instead of the negative number (this is only temporary until I increase altitude above 0m).

Btw. I have one question, do we still need to keep the accelerometer enabled ? Or can I disable it ? I left it enabled because I wasn't sure whether your code is still using it in cases when I fly too slow or change directions erratically.

Anyway, very happy with the test results so far, it was very easy to find the home, every time.

If it's allowed, I may upload the DVR footage with OSD and post a link here.

Cheers

mikeller commented 6 years ago

Btw. I have one question, do we still need to keep the accelerometer enabled?

You don't have to. But if enabled, the accelerometer data will be used to improve the accuracy of the altitude data, so without it the altitude information will be less accurate.

AirBreak69 commented 6 years ago

@peteoz Thanks a lot for detailed report. Looks better than expected. My code does not change the existing functionality. It simply activates the altitude/heading calculations in presence of GPS on non fixed wings too. The strategies are as they ever were...

Currently the heading is only used above 3m/s speed. Maybe this could be lowered to catch earlier.

Congrats, you are living by the sea - msl is near. Some variables inside the NMEA parser are unsigned. They tend to underflow. But negative altitudes seem not to be handled at all. Should be changed. -0.7m must be from the double integrated Z acceleration. Without accelerometer the displayed altitude should be identical to the pure GPS altitude with 1m resolution.

AirBreak69 commented 6 years ago

@mikeller I aggree to you: Posting ZIPs and tracking changes becomes annoying to me, too. A desperate action because I wasn't successful in fixing my GIT jam. If I'am allowed: I'll try to contact you via slack the next days with more detailed descriptions.

mikeller commented 6 years ago

Sure, happy to help.

peteoz commented 6 years ago

@AirBreak69 here is some DVR footage from yesterday that you can analyze.

for both flights I positioned the quad pointing South.

Flight 1

Flight 2

This was my first time flying FPV at this location and from the DVR footage it's obvious I would have trouble finding home location, but in all flights got back home easily without any issues.

Next time I go out I'll try with accelerometer disabled.

mikeller commented 6 years ago

That seems to be working quite reliably!

AirBreak69 commented 6 years ago

@peteoz Thanks for the impressive videos. The converge of the home arrow is limited by the GPS headings stability and the "attack time" of AHRS stabilization. Under ideal conditions i noticed around 15 to 20s. Under strong pitch/roll changes the GPS receiver stumbles (your videos show satellite-count dropping from 21 to 10). Looking at the IMU code I found that the parameter imu_dcm_kp (default=2500) controls the global amplification of the 3 axis stabilization. In our case the Z axis is guided by the GPS heading. You may try to carefully rise the value. I guess doubling should half the converge time. But I never touched this...

AirBreak69 commented 6 years ago

@peteoz Here's a special build for you that should address the underflow problem on negative altitudes. Airbreak69_PreRelease_for_CLRacingF4_20170114_based_on_build_1959.zip

The matching speed and quality of the home arrow can hardly be improved - sorry:

peteoz commented 6 years ago

@AirBreak69 thank you. and just to clarify, the quality of home arrow is very good as it is, it allows me to bring the quad home. it's understandable there is no information on takeoff but as soon as I start heading some direction it locks in and works very well for the rest of the flight, and it even reacts quite fast to changes of direction (0.5 to 1sec). likewise quality of the baro function is very good, I don't really need altitude info unless I fly in the mountains but it's reasonably accurate (I mean if I'm at sea level and it shows -5m, that's for me within very acceptable margin of error).

honestly, we are talking about a tiny micro GPS unit that has no baro, nor compass, so I'm very impressed with functionality as it is.

I installed version based on build 1959 but didn't get a chance to test it yet, we had some bad weather with 40-50km/h winds, I'll go out again on Wednesday.

also just to let you know few other observations that I believe have nothing to do with your code changes and are probably related to the build you base your changes on

peteoz commented 6 years ago

@AirBreak69 just a quick report - I went to the same location last week and made 6 flights. the home arrow seems to be working same as already reported, meaning I'm very satisfied with the results (it takes little while to lock in when I take off pointing South instead of North) and after that it's very accurate, responds to changes in direction and does the job in allowing me to find home location easily. the baro function also works quite well and the underflow error for negative altitude was also fixed, I can confirm that.

there is one minor issue that is now new in version b1959 in that the GPS coordinates don't show properly on the OSD screen, some of the numbers are blocked, this was not an issue in previous version b1942, please see the attached screenshot to see what I mean. (if you want me to upload the dvr video as well, just let me know)

image

everything else I have already reported in my last update above.

all I can hope for is that your code changes can be successfully submitted and eventually make it to a release candidate. the same functionality already exists in current BF official release, just that this is a better implementation of the same functionality (in my opinion)

thank you

AirBreak69 commented 6 years ago

@peteoz Thanks a lot for your detailed reports. As I stated before the functionality couldn't be improved without additonal sensor fusion and intelligence. But the current state is better than I expected when starting the project. The glitches in the GPS coordinate display look strange and cannot be forced by my changes. Currently the central OSD code is identical to the general code base. You might rise a general issue. Another idea: check if you configured other elements overlaying the coordinates. I asked one of the project members to publish my code changes because I was unable to fix my GIT problems. I hope we can get the thing flying with some amount of external assistance... Thank you!

beeb commented 6 years ago

Hello all! Dropping by to ask the current status of this? I didn't see any mention on the changelog for 3.3RC1 so my guess is that it hasn't been merged yet. Can I help in some way? I'm not too savvy with git but I can try.

EDIT: read the PR discussion, too bad it didn't make it into 3.3, the wait to 3.4 will be a long one. Also read that some unnamed guy is apparently keen on implementing this again, so I don't think my help is necessary, knowing I'm not familiar with the code base.

mikeller commented 6 years ago

@beeb: There is something for this in the works, but it was not ready for the cut off for 3.3, so it will go into 3.4 (if it is ready by then).

mikeller commented 6 years ago

@wx4cb: Can you please confirm that this has been solved, and close the ticket?

wx4cb commented 6 years ago

i have no way of testing this right now as i don't have a build with betaflight on and a gps. I would suggest go ahead and close it for now. No point in having this stale