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

New OSD features #1263

Merged
merged 2 commits into from Oct 5, 2016
Merged

New OSD features #1263

merged 2 commits into from Oct 5, 2016

Conversation

DanNixon
Copy link
Member

@DanNixon DanNixon commented Oct 3, 2016

  • Added max. altitude to statistics screen
  • Allow the crosshair to be toggled separately to the artificial horizon

@nathantsoi
Copy link
Contributor

looks good @DanNixon, also saw the configurator pr. unless there are any objections, I think we could pry get these into master w/ the other osd changes

also, not about this pr but related: i was thinking about putting the osd default layout back to something similar to how it was before? i know you didn't change this, but it might make the upgrade smoother for folks.

@DanNixon
Copy link
Member Author

DanNixon commented Oct 4, 2016

@nathantsoi Just checking, by default layout you mean the default enabled items and their position?

I agree this should be as similar as possible. Another possible option would be to try to match MW-OSD as well since all the OSD items used there exist here now and that's probably what a lot of people would have previously used.

@DanNixon
Copy link
Member Author

DanNixon commented Oct 4, 2016

@nathantsoi I'll make the changes to go into master now (if nothing else it gets rid of the extra version check in the configurator).

Edit: done, just needs testing on hardware (should get round to it by the end of today).

@DanNixon DanNixon changed the base branch from development to master October 4, 2016 08:08
@jflyper
Copy link
Contributor

jflyper commented Oct 4, 2016

I will submitting an experimental TBS Unify 5G8 Pro (SmartAudio) support shortly, that currently (as a PoC code) controllable from bfosd from 3.0.0.

I'm looking forward to working with you folks.

IMO, the vtx system is a mess; we already have singularity, sirin, and smartaudio is joining shortly. We need to come up with a proper abstraction (I believe I saw some work in bfosd code updates from 3.0.0 to support singularity stuff, I'm not sure.), and code consolidation (there are frequency tables and RNA register math everywhere).

And then, osd menu architecture that can support device dependent menu page would be super. I believe the current code can be extended to support it; not a single array, but linked list of pages, may be.

Please take a look at the second comment in #1029 also.

@DanNixon
Copy link
Member Author

DanNixon commented Oct 4, 2016

@jflyper Completely agree with your comments, there should just be a single interface for VTXs then all existing ways of channel configuration should just work.

I feel the same about the OSD video hardware, there are a couple of potential replacements for the MAX video IC and it would be nice to have the OSD code more decoupled from the video hardware than what it is now. This is something I planned to work on over the next couple of months.

ps. looking forward to having SmartAudio too, been eyeing up a TBS Unify for my next build.

@martinbudden
Copy link
Contributor

@DanNixon , @jflyper , I agree the OSD code is currently quite tightly coupled to the video hardware - it would be good to see this abstracted.

@hydra has implemented a more general OSD for Cleanflight at https://github.com/cleanflight/cleanflight/tree/master/src/main/osd - it would be good to see this ported to betaflight, or at the very least form the basis for your work.

@advancetom
Copy link

Most use micro minim OSD

@DanNixon
Copy link
Member Author

DanNixon commented Oct 4, 2016

@nathantsoi Gave this an extra test and it seems fine.

@martinbudden Yes, I quite like the way the OSD is done in Cleanflight, I'd be happy to try and get this ported to Betaflight. (probably without the ability to compile the OSD as a standalone device)

@borisbstyle
Copy link
Member

@DanNixon
This has to go to development branch

@jflyper
Copy link
Contributor

jflyper commented Oct 4, 2016

@advancetom
When the configuration menu system gets a separation from osd code, say, by abstracting the osd as XY-addressable display device, then it would be an easy task to support external display devices through appropriate MSP extensions. I already have a branch of MWOSD that supports such MSP extension, prepared for cleanflight/cleanflight#2014 which was originally intended for I2C OLED display device.

@nathantsoi
Copy link
Contributor

i just finished testing on hardware. works great! also, thx for the configurator fix

@nathantsoi nathantsoi merged commit 9d4c240 into betaflight:master Oct 5, 2016
@jflyper
Copy link
Contributor

jflyper commented Oct 6, 2016

Informational PR submitted as #1275.

It can be tested only by those with OMNIBUS F3 AND Unify PRO...

@jflyper
Copy link
Contributor

jflyper commented Oct 7, 2016

@nathantsoi
Wasn't this to go to development?

@jflyper
Copy link
Contributor

jflyper commented Oct 7, 2016

@DanNixon
So, with this PR, FC restarts when exiting from the menu, right?

Is this temporary, or final design?

@nathantsoi
Copy link
Contributor

nathantsoi commented Oct 7, 2016

i think this pr is actually just improvements and fixes for
#1193 (also #1218), which were in master

git blame will show for sure, but i believe @marbalon would know the
answer to the reboot question.

@DanNixon
Copy link
Member Author

DanNixon commented Oct 7, 2016

@jflyper The restarting is intentional, some setting changes would require a restart (i.e. feature enable/disable), however there is certainly no reason to restart when discarding changes.

Is the rebooting causing problems?

(FWIW this PR did originally base development)

@jflyper
Copy link
Contributor

jflyper commented Oct 7, 2016

Am I on the wrong PR?

@DanNixon
Yes, I understand there are changes that must accompany restarts, but I think pilots would complain about obviously unnecessary restarts. Those parameters that doesn't need restart with configurator fall into this category.

I can't immediately come up with a case that restarting would cause a problem, but in my experience, there are some. There were peripheral devices that required "soft state" to be kept in the main memory, and the system just couldn't be restarted without power-cycling these devices.

Mark items requires restart and change "Save&Exit" to "Save&Reboot" upon modification?

@DanNixon
Copy link
Member Author

DanNixon commented Oct 7, 2016

You can change the base of a PR (usually requires a manual git rebase) once opened, thats what I did here.

I completely agree about the rebooting, I think always doing a reboot just because it may be needed in some cases is a hacky way to solve the problem.

After when parameter groups are added I want to port the Cleanflight OSD and work on a menu system using that as a starting point.

@jflyper
Copy link
Contributor

jflyper commented Oct 7, 2016

@DanNixon
Can you look at cleanflight/cleanflight#2014, too?
I'm going to fiddle with it this weekend to make it device-quasi-independent, so that the it can talk to an external osd via a new MSP message, say, MSP_CANVAS(x, y, string) or something similar.

@jflyper
Copy link
Contributor

jflyper commented Oct 19, 2016

@DanNixon
Have you modified the OSD code (io/osd.c) lately? I've done another PoC work on OSD Menu - Display device separation. It now can drive external OSD (modded MWOSD) for config menu.

@DanNixon
Copy link
Member Author

@jflyper I'm afraid I haven't had a chance to do any more lately. I'm still interested though, I assume your work builds on the Cleanflight OSD?

@jflyper
Copy link
Contributor

jflyper commented Oct 19, 2016

Good that you haven't done additional work, because my code is based on yours (naturally, it is isolated from CFOSD code).

I heavily hacked up your code (I call it BFOSD), separated OSD and CMS (Configuration Menu System) that calls a "screen" device abstraction to layout menu items. The screen device can be configured as Max7456 or external UART-based OSD (I modded MWOSD).

I will submit a discussion PR shortly. It's going to be great if take a look at it and add some comments on your spare time.

@DanNixon
Copy link
Member Author

@jflyper Cool, does your "screen" operate based on character positions or some other coordinate system (e.g. pixels, normalised device coordinates, ...)?
I had been playing around with software video generation and that was my main reason for wanting to have the video hardware abstracted away from the OSD logic.

@jflyper
Copy link
Contributor

jflyper commented Oct 19, 2016

It's #1352 .

The current "screen" is character based canvas; I only have access to MimimOSD and variants.
Protocol is very simple, something like drawStringAt(row, col, string).

@hydra
Copy link
Contributor

hydra commented Oct 19, 2016

@DanNixon the CF is a better base if you want an abstraction layer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants