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

Configurator 3.0.0 - Mission Control - Load Eeprom mission button #1270

Closed
ayoaina opened this issue May 21, 2021 · 25 comments
Closed

Configurator 3.0.0 - Mission Control - Load Eeprom mission button #1270

ayoaina opened this issue May 21, 2021 · 25 comments

Comments

@ayoaina
Copy link

ayoaina commented May 21, 2021

In Configurator 3.0.0 on Windows, when you click on the Load Eeprom mission button, nothing happens. You have to click on it a second time before it loads the mission.

@ayoaina
Copy link
Author

ayoaina commented May 21, 2021

Update on this issue:
It appears that it works OK the first time you load eeprom mission after opening the Mission Control tab.
You get the issue if you delete the points and then try loading again.

@Jetrell
Copy link

Jetrell commented May 22, 2021

@ArnoTlse Is this edge case fixed in #1267 ?

@ArnoTlse
Copy link
Contributor

Hi,

no, not in this one, Eeprom loading was corrected in a former PR.
I will try to reproduce this sequence with my current dev version.

Arno

@ArnoTlse
Copy link
Contributor

Hi @ayoaina , hi @Jetrell ,

On the last version 3.0.0, It seems to work fine on my side.
The PR which corrects this seems to have been included.

Are you still facing issue with the last 3.0.0 version ?

If not, can we close this Issue ?

Regards

Arno

@Jetrell
Copy link

Jetrell commented Jun 20, 2021

@ArnoTlse I found the issue. It occurs when nav_wp_load_on_boot = ON is used.

Steps to reproduce. With a mission already saved to the eeprom and nav_wp_load_on_boot = ON enabled.

  • Start configurator
  • Plug in FC to USB (no flight battery used)
  • Open MP and press load eeprom mission once. Five seconds later Bing maps image disappears.
  • Unplug FC from USB (no flight battery used). Then restart the configurator and power FC again.
  • Open MP and the Bing maps image is still not there.
  • Click on load mission from FC then the mission and Bing maps reappears.

Set nav_wp_load_on_boot = OFF and the mission loads from the eeprom with one click.

@tonyyng Have you experienced this?

@tonyyng
Copy link

tonyyng commented Jun 20, 2021

@tonyyng Have you experienced this?
No, I haven't experienced this. I'll have a look when I get a chance.

  • Open MP and press load eeprom mission once. Five seconds later Bing maps image disappears.

What happens if you press load eeprom mission twice at this point? Maybe the MP logic is assuming there is no mission loaded when it first connects.

@Jetrell
Copy link

Jetrell commented Jun 20, 2021

  • Open MP and press load eeprom mission once. Five seconds later Bing maps image disappears.

What happens if you press load eeprom mission twice at this point?

With nav_wp_load_on_boot = ON, if pressed twice it loads. And once, bing maps disappears.
With nav_wp_load_on_boot = OFF if pressed once it loads.

Maybe the MP logic is assuming there is no mission loaded when it first connects.

Yes, That is confirmed by pressing load the mission from FC after connection, with nav_wp_load_on_boot = ON.
While it will not load from that button when nav_wp_load_on_boot = OFF, as expected.

@tonyyng
Copy link

tonyyng commented Jun 20, 2021

The fix should be made in the Configurator code, which I'm not familiar with.

@ArnoTlse
Copy link
Contributor

Hi,

I'll try to investigate why there is a difference when auto load on boot is ON. I don't see why it should change the behavior from eeprom as eeprom is only meant to be read when user request it. Maybe there is a change in the MSP protocole when auto load on boot is active?

In the same time, I will look at the ability to switch on/off the auto load on boot in the settings and
If load on boot ON, I'll look if it could be automatically load the mission from eeprom when FC is connected. If no FC connected or FC connected and load on boot OFF, no try. It is up to the user to load it through the button.

Arno

@ArnoTlse
Copy link
Contributor

Hi,

I guess there have been a change in INAV FC 3.0 that trigger this issue now on the Configurator. I have not this issue on a previous 3.0.0 master version of my FC. The issue has appeared since the last release of INAV 3.0.0_final.

With INAV 3.0.0_final release on my FC, when you load mission from FC, you can do it many time as you want, it loads perfectly. However, I reproduce the issue when I hit the Load from EEPROM button ; it works the first time. The second time, it brings you in 0,0 lon-lat location. The third time, it loads well the mission, the fourth, it brings you again at 0,0 lon-lat location, etc, etc ...
The problem is either with Bing or Openstreet map, or either nav_wp_load_on_boot = ON or OFF.

So my main guess is that there have been a change in INAV FC way of loading EEPROM on the latest version of INAV 3.0.0. I have to investigate more on that to see what changes on INAV FC lead to this issue on Configurator now.

Arno

@tonyyng
Copy link

tonyyng commented Jun 25, 2021

I made this change which was pulled into 3.0. As part of that change, this function changed so that if you call it when waypoints are already loaded, it will clear them.

bool loadNonVolatileWaypointList(void)
{
    if (ARMING_FLAG(ARMED))
        return false;

    // if waypoints are already loaded, just unload them.
    if (posControl.waypointCount > 0) {
        resetWaypointList();
        return false;
    }

I'm pretty sure that is the behavior you are seeing. When you clock Load the second time, the waypoints are being cleared.

Can MP be changed so that it only executes Load if there are no waypoints in the FC memory? (posControl.waypointCount == 0)

If you want to force this function to load the waypoints, call resetWaypointList() before executing the Load logic.

@ArnoTlse
Copy link
Contributor

Hi @tonyyng,

Ok thanks for the information.
I have to figure out how it has been implemented then on the MSP code as I use it to load the mission from Eeprom. So I guess MSP way of working for loading eeprom have also changed according to this evolution ?

On MP side, as a first guess, I would prefer that when user hits load from Eeprom, it loads really the mission from Eeprom. If we take into account whether a mission is already loaded or not, I fear we cannot say of we have to update the load if mission has been updated between by a save.

@stronnag, I wonder if you have experimented this also on MWP?

Arno

@stronnag
Copy link
Collaborator

@tonyyng's change should be reverted or updated such that MSP_WP_MISSION_LOAD works as before (i.e. unconditionally loads WPs from EEPROM). Breaking legacy APIs is not acceptable behaviour.

@TonyNg, please fix this.

@stronnag
Copy link
Collaborator

@stronnag, I wonder if you have experimented this also on MWP?

I have, and it's not so much of a problem, as mwp informs the user that there are WPs in memory (that does not however condone breaking of the MSP_WP_MISSION_LOAD API).

@stronnag
Copy link
Collaborator

@tonyyng , please test that iNavFlight/inav#7190 doesn't cause any issues for you. This restores legacy behaviour for MSP_MISSION_LOAD (and loading via stick command). In the latter case, the user may be flying LOS would expect the WPs to be loaded as previously.

@ArnoTlse
Copy link
Contributor

Ok, thanks.

Good point, I'll try to improve the way the user is informed about missions already available in the FC. Today, there is a green/red flag to indicate if a mission has been properly saved into, but not indeed a complete indicator of mission status (lack a status when FC is connected or configurator is launched).

@stronnag
Copy link
Collaborator

@ArnoTlse MSP_WP_GETINFO on connect or tab load maybe

@tonyyng
Copy link

tonyyng commented Jun 26, 2021

@tonyyng , please test that iNavFlight/inav#7190 doesn't cause any issues for you. This restores legacy behaviour for MSP_MISSION_LOAD (and loading via stick command). In the latter case, the user may be flying LOS would expect the WPs to be loaded as previously.

The objective of my change was to avoid forgetting to load waypoints from EEPROM before launching. When you forget, you have to land, disarm, load waypoints and then launch again before you can fly waypoint missions.

With nav_wp_load_on_boot = ON, waypoints are automatically loaded during FC startup.

However, this raises an issue if you are flying at a different field. If the waypoints are loaded from EEPROM and the first WP is too far, you won't be able to arm. That was the reason for changing the stick command to be a toggle. By reverting that part of the change, there is no way to unload the waypoints and arm.

I suggest the following:

Revert my change in loadNonVolatileWaypointList(). It will go back to always loading waypoints and the Configurator issue will be resolved.

    // if waypoints are already loaded, just unload them.
    if (posControl.waypointCount > 0) {
        resetWaypointList();

Then change the stick control logic:

    // Unload or load the waypoint list
    if (rcSticks == THR_LO + YAW_CE + PIT_HI + ROL_HI) {
        bool loaded;
        if (posControl.waypointCount > 0) {
            resetWaypointList();
            loaded = false;
        } else {
            loaded = loadNonVolatileWaypointList();
        }
        beeper(loaded ? BEEPER_ACTION_SUCCESS : BEEPER_ACTION_FAIL);
    }

I'm in the middle of house renos, so I really can't spare time to do much more. @stronnag I see you've gone ahead an made some changes. Can you add these changes? Also, I'm not sure I've addressed the problem you've described here. Can you comment?

the user may be flying LOS would expect the WPs to be loaded as previously.

@stronnag
Copy link
Collaborator

stronnag commented Jun 27, 2021

Your proposed code would not compile as poscontrol is private to the nav subsystem. At the moment, the merged solution reverted the stick toggling of WP load /unload (which is sub-optimal with load-on-boot).

Tomorrow, I'll enhance the 3.0.1 branch to:

  • Add a public missionLoaded() API (if there is not one somwhere already). Use the public getWaypointCount()
  • Re-implement stick toggle of load / unload
  • Use the beeper to only indicate success if the WPs are loaded into non-volatile memory (to provide some indication to non-OSD (LOS) pilots of state).
  • Update the documentation images to show load / unload for the stick command

@tonyyng
Copy link

tonyyng commented Jun 27, 2021

That sounds good. Thanks

@ArnoTlse
Copy link
Contributor

Hi,

I have a question about MSP. How can we clear the RAM or the EEPROM of a mission with MSP ? Is it just by sending an empty mission or is there a MSP code to clear a mission?

Arno

@stronnag
Copy link
Collaborator

Hi,

I have a question about MSP. How can we clear the RAM or the EEPROM of a mission with MSP ? Is it just by sending an empty mission or is there a MSP code to clear a mission?

Arno

Upload an empty mission (i.e a single RTH WP). Save to EEPROM.

@ArnoTlse
Copy link
Contributor

OK thanks.

I'll add a dedicated button in the Configurator to clear a mission in this way.

Arno

@ArnoTlse
Copy link
Contributor

Hi,

I wonder whether we could close this issue now?

Regards

Arno

@tonyyng
Copy link

tonyyng commented Sep 20, 2021

Hi,

I wonder whether we could close this issue now?

Regards

Arno

Okay by me

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

No branches or pull requests

6 participants