Configuration Menu System (CMS)

[jflyper]

This is a continuation of #1352.

With enormous help from the community, the CMS is now:

• Partially separated from OSD; more separation needs CMS data structures to be moved to osd.c.
• Capable of running on multiple display devices: OSD, UART connected external OSD (MWOSD with CANVAS extension) and I2C OLED; if multiple displays are available simultaneously, they can be switched from one to another.

Integrated OSD & External MWOSD

I2C OLED

Code branch: https://github.com/jflyper/cleanflight/tree/bfdev-osd-cms-separation-poc

MWOSD: (already merged in the current master) https://github.com/ShikOfTheRa/scarab-osd/tree/master

[jflyper]

I am having some trouble with MSP injection behavior. When configurator is connected via VCP, messages are sent back to back as they are generated. However, when the configurator is not connected, it looks like injected MSP messages are spaced 150msec a part, and a sender looks like blocked.

I thought the tx side was interrupt driven, but the above behavior suggest it's clocked or triggered by received MSP messages.

[jflyper]

@llambkin Over here. BF branch and corresponding MWOSD branch is at the bottom of the first comment.

[llambkin]

Cool, I just had a look at the target.c for the REVO, it looks like it has the D-shot code in there :D Thanks..... I will build myself a hex and give it a go :D

On Sat, Oct 29, 2016 at 2:31 PM, jflyper notifications@github.com wrote:

@llambkin https://github.com/llambkin Over here. BF branch and corresponding MWOSD branch is at the bottom of the first comment.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/betaflight/betaflight/pull/1422#issuecomment-257074464, or mute the thread https://github.com/notifications/unsubscribe-auth/ADB9tR-QwqTObzxPFdLMwQw_rjF3X33uks5q4ugzgaJpZM4Kjqkb .

[jflyper]

Oops. I tried it with OMNIBUSF4, which is similar to REVO, and it hangs as I tried to go into the menu without connecting to the configurator. (FC it self is still functioning.)

There must be something wrong with the MSP injection.

[llambkin]

Ok, no problems, let me know when you have something to test, thanks for letting me know. I was going to do my build in a few hours when my toddler goes to bed.

On 29 Oct. 2016 3:29 pm, "jflyper" notifications@github.com wrote:

Oops. I tried it with OMNIBUSF4, which is similar to REVO, and it hangs as I tried to go into the menu without connecting to the configurator. (FC it self is still functioning.)

There must be something wrong with the MSP injection.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/betaflight/betaflight/pull/1422#issuecomment-257076722, or mute the thread https://github.com/notifications/unsubscribe-auth/ADB9tWpSaH50SDJ9AOw_iJmb_6slN9Cwks5q4vW_gaJpZM4Kjqkb .

[jaapp]

With the new work underway of bridging the Taranis to the flightcontroler via smartport - allowing MSP communication via a LUA script on the Taranis ( https://github.com/betaflight/betaflight/issues/1311 ) it could be possibile to leverage the Taranis itself as a interface for the Configuration Menu System (CMS). Is that something that is possible with the CMS as it is designed now?

[jflyper]

@jaapp It's certainly feasible. There has been a similar work with HoTT telemetry. With lua speaking taranis, it would be much easier.

[jflyper]

It took me full weekend to find out why my CMS on CANVAS gets 'clogged' when VCP MSP is not connected. I haven't looked into the details of how VCP disconnection is reflected into corresponding mspPorts[] element, but it certainly look like writing to non-NULL mspPort->port with invalid (non-active?) VCP connection takes order of tens of milliseconds to process in the following code.

void mspSerialPush( ... )
{
    for (uint8_t portIndex = 0; portIndex < MAX_MSP_PORT_COUNT; portIndex++) {
        mspPort_t * const mspPort = &mspPorts[portIndex];
        if (!mspPort->port) {
            continue;
        }

        ...

        mspSerialEncode(mspPort, &push);
    }
}

Fully avoiding VCP port pragmatically solves my problem.

void mspSerialPush( ... )
{
    for (uint8_t portIndex = 0; portIndex < MAX_MSP_PORT_COUNT; portIndex++) {
        mspPort_t * const mspPort = &mspPorts[portIndex];
        if (!mspPort->port) {
            continue;
        }

        // XXX Kludge!!! Avoid zombie VCP port (avoid VCP entirely for now)
        if (mspPort->port->identifier == SERIAL_PORT_USB_VCP) {
            continue;
        }

        ...

        mspSerialEncode(mspPort, &push);
    }
}

[jflyper]

@llambkin It now works on OMNIBUSF4 (REVO-like) also. Add CMS and CANVAS to target.h, and add io/cms.c and io/canvas.c to target.mk.

[raphaelcoeffic]

@jflyper: do you think it would be possible to run these config menus over a 1k-2k bps link? Because that's all we get over SPORT. by the way, what would it take to support this over SPORT? would the rendering code have to be in LUA? ATM, the MSP/SPORT bridge does not push anything, only replies. However, it should be fairly easy.

[jflyper]

@raphaelcoeffic

1. 1K-2K bps link should be no problem; when the CMS was separated from the original OSD code, it was modified NOT to draw unmodified part of the screen, and was also modified to be bandwidth sensitive. However, I haven't tested it at that low speed yet. (Some functions with relatively high bandwidth demand such as RC channel preview may have to dropped, though.)
2. My guess is that the MSP over S.PORT will just appear as a port in mspPorts[] array, with different underlying transport. If that's the case, a serializer is required for the transport. The MSP layer code should be re-usable. See mspSerialPush() in msp/msp_serial.c and mspServerPush() in fc/fc_msp.c for the current implementation (It will be changed a bit to follow recent changes in msp/serial separation, though).
3. A device has to interpret couples of commands: begin, end, clear, draw(x, y, string) to support CANVAS extension. See io/canvas.c for details (there isn't much details there:))

Additional concern I have is the device selection (or use case). Currently, the CMS assumes there is only one CANVAS enabled external device (i.e. MWOSD) in the system, on UART. If the MSP over S.PORT will co-exist with the external device, we have to come up with some way to distinguish these two. A new SCANVAS device that will also detect and handle an MSP over S.PORT may be our first solution.

Once the MSP over S.PORT code is working, I can help you to get above done. Only problem is that I don't have a taranis...

[raphaelcoeffic]

It might be a silly proposal, considering that you're basically done with your feature, but couldn't we just transport the menu structure / options (cacheable on the client side), so that the UI is more reactive? I'm really afraid to be remembered the time a serial terminal connected with a 2400kpbs modem was a top new technology. Plus, we could store that menu structure + options on the SD card present in the taranis. Does that make any sense, or is that way off reality?

[edit] I realise that this would push the memory requirements on the client side quite some. Whereby this is not an issue on the taranis (whereby LUA has some memory limits), this will be constraining on an atmega 328. Those who want to save memory will have to have more bandwidth, but that reflects the reality.

[jflyper]

As you already have noticed, having those menu data structures (let's call them contents) over at the client side will have severe pressure on the memory usages on M328 based devices, with the data itself and the marshalling/demarshalling code that handles pointers across multiple devices with potentially different MCU architecture.

Another point is that it will take substantial time to synchronize/cache/pre-fetch/download the menu contents from the FC to the device.

Why don't we just do the direct draw approach first and see how it behaves? IMO, it's not that hard to do this.

[raphaelcoeffic]

Ok, then, let's try it your way first. That would save me from developing a UI on the taranis :-) Are the command only possible with RC sticks? Or could I just send the keys from LUA?

On 1 Nov 2016, at 04:11, jflyper notifications@github.com wrote:

As you already have noticed, having those menu data structures (let's call them contents) over at the client side will have severe pressure on the memory usages on M328 based devices, with the data itself and the marshalling/demarshalling code that handles pointers across multiple devices with potentially different MCU architecture.

Another point is that it will take substantial time to synchronize/cache/pre-fetch/download the menu contents from the FC to the device.

Why don't we just do the direct draw approach first and see how it behaves? IMO, it's not that hard to do this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jflyper]

With sticks only (as far as my handy devices are capable of).

It should respond to RX_MSP/MSP_RC(whatever), though. How do your keys appear as inputs inside BF?

[DanNixon]

@raphaelcoeffic Having to have the client and server code in sync would just make maintenance more difficult that it needs to be.

A better solution if bandwidth is really that much of an issue would be to have the client request an array of strings on connection and have a separate draw string function that provides an index in this array. Since pretty much all of the text is predefined there shouldn't be too much issue with this.

It would be possible to have the keys just perform stick gestures, but I'm not overly keen on that as accessing the menu requires non zero throttle and it would be easy to accidentally switch to this (assuming implementation as a telemetry screen) when armed.

[jflyper]

A better solution if bandwidth is really that much of an issue would be to have the client request an array of strings on connection and have a separate draw string function that provides an index in this array. Since pretty much all of the text is predefined there shouldn't be too much issue with this.

Things to consider in this approach:

1. Latency at menu start or menu page transition. How long does it take to download strings for all or the current page.
2. If lua has local storage, then strings can really be cached, and string ordinals can be invalidated by version info of string sets.

[DanNixon]

@jflyper Yes, if it was cached and versioned there shouldn't be too much of a problem with latency.

I wouldn't expect it to be cached when changing pages, ideally it would be done the first time the menu was invoked. (although would caching strings for a given page when it is opened take much longer then just rendering the page normally?)

[DanNixon]

Another bandwidth saver could be to send numerical values as numbers and do the string formatting on the client.

The format string could be set as a state of the display device and all subsequent calls to draw number would use this format string. There would have to be handling of cases where a floating point number is represented as an integer (IRC PID and rate values are like this?).

This could be an optional addition for devices where bandwidth is actually an issue (i.e. external Minim OSD and MSP-S.PORT).

[jflyper]

I searched the net to find out the screen resolution of the taranis, which is 212x64 dots. If we fit 5x7 glyph in 6x8 grid, then it would be 35 cols x 8 lines, or 280 chars. Suppose we fill average of 70% of the screen with chars (look at the OLED case in the first comment), it would be 196 chars. At 1Kbps/100cps, it then would take 2secs to draw the new menu screen. At 2Kbps, 1sec. Cursor and value changes would be almost instantaneous.

We have to evaluate these numbers against software complexity and use cases.

EDIT: I forced the CANVAS on MWOSD (and associated serial port) to operate at 1200bps to get a feeling of what it is like to operate the CMS at that speed. Here's how it looks like:

cms@1200bps.zip

[raphaelcoeffic]

On 1 Nov 2016, at 08:01, Dan Nixon notifications@github.com wrote:

@raphaelcoeffic Having to have the client and server code in sync would just make maintenance more difficult that it needs to be.

Don't know where you read that. The rest of your comment describes more or less what I meant. I should probably learn to better explain myself.

A better solution if bandwidth is really that much of an issue would be to have the client request an array of strings on connection and have a separate draw string function that provides an index in this array. Since pretty much all of the text is predefined there shouldn't be too much issue with this.

Exactly. It would be possible to have the keys just perform stick gestures, but I'm not overly keen on that as accessing the menu requires non zero throttle and it would be easy to accidentally switch to this (assuming implementation as a telemetry screen) when armed.

That's an issue, apart from which: it feels quite stupid navigating through menus on your transmitter with the sticks. Like shifting the gears with your rear mirror.

[DanNixon]

@raphaelcoeffic I must have misunderstood "but couldn't we just transport the menu structure / options (cacheable on the client side), so that the UI is more reactive?" to mean move the UI code to the client rather than just cache the UI in the client.

Reading it again it is pretty obvious what you meant.

[raphaelcoeffic]

On 1 Nov 2016, at 09:41, jflyper notifications@github.com wrote:

I searched the net to find out the screen resolution of the taranis, which is 212x64 dots. If we fit 5x7 glyph in 6x8 grid, then it would be 35 cols x 8 lines, or 280 chars. Suppose we fill average of 70% of the screen with chars (look at the OLED case in the first comment), it would be 196 chars. At 1Kbps/100cps, it then would take 2secs to draw the new menu screen. At 2Kbps, 1sec. Cursor and value changes would be almost instantaneous.

We have to evaluate these numbers against software complexity and use cases.

Another thing to consider is that the client transmits only scarcely. When it comes to SPORT, you can only send within your slot. If you do not send packets all the time like the FC does, it can take quite some time until you get a slot on the transmitter side (up to 1s), which is why I'd to avoid "remote controlling" the cursor and value changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jflyper]

You can stop the telemetry polling / pushing while menu screen is active. That's what MWOSD in canvas mode do. There's no point sending telemetry data if no one sees them.

[raphaelcoeffic]

@jflyper "You can stop the telemetry polling / pushing while menu screen is active. That's what MWOSD in canvas mode do. There's no point sending telemetry data if no one sees them."

That would not exactly help, as you loose your slot if you don't use it. Then, you need to wait until the receiver/transmitter polls you again to regain a permanent slot. SPORT is a bit of a pain in that matter. So basically, it is best when the FC keeps sending data over SPORT. However, it does not make sense to have the same at the transmitter side, as it would divide the available bandwidth for the receiver side by almost 2.

[martinbudden]

@jflyper , this is now much improved. Let's finish cleaning it up so that it can be included in 3.1. This will probably need a few more iterations of cleanup, but it is getting close.

[martinbudden]

@jflyper , can you rebase? I'd like to make a PR against your branch - easier to show you the code than explain the changes.

[jflyper]

@martinbudden It's rebased.

Large change has been done trying to get device/feature specific menu contents out of the cms.c. This required OSD_Entry arrays to be declared as externs in their relevant header files. The cms.c still has some very nasty device/feature specific references to externally supplied menu contents; I'm thinking how to get rid of these.

[llambkin]

I have been testing D-shot with my setup, and it appears that something corrupts the config when PIDs are changed. My copter was working as per usual. I went into the CMS, and changed the PIDs, hit save, and now the copter doesnt respond to the throttle channel and the cyclic either. I have had it where it reacts to movement when the throttle is 0, but if I move the throttle up, it just returns to idle and doesnt correct for errors any more.

This continues on from what MWOSD does. When I last tried to change settings in MWOSD it did the exact same thing, I hit save and then the ESC's started beeping, And didnt stop until I unplugged the battery.

On the plus side, the CMS works flawlessly via the sticks, there is no delay when moving the cursor around the screen.

For now, I will have to reset settings to default and stick to using BF configurator to edit my PIDs

[jflyper]

@martinbudden I'm experiencing an issue that entering cli hangs entire FC after the last rebase. I then tested master branch and got the same result with targets OMNIBUS, OMNIBUSF4 and SPRACINGF3. I'm going to submit a new issue.

[jflyper]

@llambkin Thanks for the report! I'll look into it!

[martinbudden]

@jflyper , I'll try and look into CLI issue.

I'll also try and get a PR for you code with my suggestions in it.

[jflyper]

@martinbudden I suspect it's the disabling -flto PR... Weird...

[martinbudden]

See PR https://github.com/jflyper/cleanflight/pull/2

[martinbudden]

@jflyper , I've been thinking about how the cms device registration works, and I think it could be improved by registering the displayport, rather than the init function. So, for example, in canvas.c we would have:

static displayPort_t canvasDisplayPort;

void canvasInit(void)
{
    canvasDisplayPort.vTable = &canvasVTable;
    canvasResync(&canvasDisplayPort);
    cmsRegisterDisplayPort(&canvasDisplayPort);
}

and in cms.c we would have

static displayPort_t *currentDisplay;  // note this is now a pointer
...
static displayPort_t *cmsDisplayPorts[CMS_MAX_DEVICE];
static int cmsDeviceCount;
static int cmsCurrentDevice = -1;

bool cmsRegisterDisplayPort(displayPort_t *displayPort)
{
    if (cmsDeviceCount == CMS_MAX_DEVICE)
        return false;
    cmsDisplayPorts[cmsDeviceCount++] = displayPort;
    return true;
}
...
static void cmsMenuOpen(void)
{
    displayPort *nextDisplayPort;
..
    } else {
        // Switch display
        displayClose(currentDisplay);
        nextDisplayPort = cmsDeviceSelectNext();
    }

    if (!nextDisplayPort)
        return;

    currentDisplay = nextDisplayPort;
    displayOpen(currentDisplay);
    cmsMenuChange(currentDisplay, currentMenu);
}

The important thing here being that each display device initialises it's own display port, and then, if it wishes, it registers it with the CMS. That way all displays can use the displayport abstraction, and also have a choice about whether they allow themselves to be used by the CMS.

That then opens the way for all display devices to use the displayPort abstraction with all the advantages that allows - essentially anything that writes to a display will be able to write to any type of display.

[jflyper]

@martinbudden Yeap! This is close to what I was imagining initially when the displayPort_t was first introduced (comment).

[martinbudden]

Great. With that change, plus the changes in my second PR I think we are getting fairly close to being able to merge this.

[jflyper]

I'm modifying the menu structures for further separation. Device owned displayPort_t comes after this.

[martinbudden]

DYNAMIC bit is definitely an improvement.

[martinbudden]

Note that in the menu definitions you can avoid the entry items such as entryBlackboxRateDenom in cms_blackbox.c. For example:

static OSD_Entry cmsx_menuBlackboxEntries[] =
{
    {"--- BLACKBOX ---", OME_Label, NULL, NULL, 0},
    {"ENABLED", OME_Bool, NULL, &cmsx_FeatureBlackbox, 0},
    {"RATE DENOM", OME_UINT8, NULL, &(OSD_UINT8_t){&masterConfig.blackbox_rate_denom, 1, 32, 1}, 0},
#ifdef USE_FLASHFS
    {"ERASE FLASH", OME_Submenu, cmsx_EraseFlash, NULL, 0},
#endif // USE_FLASHFS
    {"BACK", OME_Back, NULL, NULL, 0},
    {NULL, OME_END, NULL, NULL, 0}
};

I think this makes the menus easier to read - all the information is in one place.

[jflyper]

Yikes! You are right!

[jflyper]

BTW, is full transition to PG scheduled for 3.1?

I'm thinking of adding several config vars; whether to use a device for CMS, preferred ordering of multiple devices, device specific parameter not available with in a system, etc.

[martinbudden]

No, I've deferred PG to 3.2. Three reasons:

1. It is quite a lot of work
2. A lot of testing is required, and it's more important that people spend time testing the new IO mapping and DSHOT
3. It's better not to have too many new things in one release.

BTW, could you rebase please?

[jflyper]

@martinbudden Ack for PG. I'll rebase as soon as the displayPort initialization mods are done; code compiled, going into testing now.

[martinbudden]

Great.

[jflyper]

Rebased. I decided to drop the led strip's color manipulation feature from the menu; there has been a big change in the led config code and didn't want to follow it. After all, I really wasn't understanding what I can do with a single entry color selection. The LED STRIP menu now only has feature on/off. Some one (including myself) with a through knowledge on led config can put it back again (with enhancements, may be).

[mikeller]

Not sure if it even makes sense to try and implement full LEDSTRIP configuration support in the OSD. After all

• it's not essential functionality in terms of flight performance, so having it misconfigured won't stop you from being able to fly safely;
• to actually be able to configure it in a somewhat intuitive way you need to be able to visualise the 16 by 16 grid, like the configurator does.

[llambkin]

Yeah I couldn't imagine the need to change up the positioning of my LEDs out in the field. But in order to have a PC-less configuration it has to be there.

On 6 Nov. 2016 4:49 am, "Michael Keller" notifications@github.com wrote:

Not sure if it even makes sense to try and implement full LEDSTRIP configuration support in the OSD. After all

• it's not essential functionality in terms of flight performance, so having it misconfigured won't stop you from being able to fly safely;
• to actually be able to configure it in a somewhat intuitive way you need to be able to visualise the 16 by 16 grid, like the configurator does.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/betaflight/betaflight/pull/1422#issuecomment-258639722, or mute the thread https://github.com/notifications/unsubscribe-auth/ADB9tVVT_pW6_BZr6zfqNpk19RG91aYpks5q7OvngaJpZM4Kjqkb .

[jflyper]

Shall we add a on screen keyboard that interfaces to serial_cli? 😎

[mikeller]

If we want PC-less configuration or feature parity with configurator we'll have to...