Enhance full parameter change message box

[prathamEndu]

Enhance the message box which shows "Parameters successfully saved" confirmation box to display changed parameters with previous and new value and count of total changed parameters.

This pull in solution to the following issue : Improved Confirmation Box for Raw Parameter Setup #3404

Here are a few images for reference :

Existing message box

Updated message box

[EosBandi]

Did you consider what happens when 50 or 100 parameters are changed after a param load or compare ?

[prathamEndu]

Hmm, I see the point
The text is clipped at the bottom, the box is larger than the screen size at the bottom, 'ok' button comes above the screen end.

Can you give me any idea how to tackle this issue??

here is an image of 150+ parameter change simultaneously :

[prathamEndu]

Is it possible to make a scrollable list or table similar to the full param table.

[EosBandi]

Yes, but I think it is too much hassle. The count of changed parameters is a good enough indicator. So I recommend to drop the list.

[robertlong13]

This confirmation should definitely come up at the "are you sure you want to write parameters" stage not the "save complete stage"

I like Andras' idea of just making it a counter. Maybe in the future, that could be extended so it could optionally launch a param compare dialog so you could review the new and old values for what you're about to write.

[prathamEndu]

I am trying the following things:

1. Make a actual confirmation box with yes/no instead of just showing it after doing the changes.
2. I will try to make a scrollable list of changes if possible
3. if "2" doesn't work then I will show the changed param lists only if <20 or <30 params are changed at a time (because these changes are probably individually done, too many changes are usually done from some param files or new config in which case we dont need to show all the params from our side)

Lemme know your view.

[prathamEndu]

The reason I am still insisting on the list is because I have been doing tuning and many times I feel that I need to see what I changed in current iteration. and hence, I believe it will be useful for others as well when manually changing params.

[robertlong13]

Not saying this is a bad idea, but you are aware about the "modified" checkbox which shows exactly what you want, right?

[prathamEndu]

Firstly, thanks for the info, I was not aware of that (and it does make sense), my bad.

and then, I still think getting a list in a confirmation box is a good idea (for less than 20 params).

anyways, this is my first time playing with mission planner code
Just as a learning exercise, I will make the changes.

If the final product looks useful, we can integrate it in the app. If not, we can leave it, I am totally fine with that as well.

[robertlong13]

Firstly, thanks for the info, I was not aware of that (and it does make sense), my bad.

Absolutely no worries.

anyways, this is my first time playing with mission planner code
Just as a learning exercise, I will make the changes.

Yup, good first project to try out. Always good to have more people who know their way around this codebase.

[prathamEndu]

The following changed are done in the final commit

In a separate confirmation box for parameter changes :

1. 0 parameter changes are ignored
2. 1-20 parameter changes are shown with details (a separate loop is run for collecting these details)
3. more than 20 parameter changes are shown just as a number

In the saved parameter box :

1. 0 parameter change message updated
2. more than 0 parameter change show the count

Preview

Fig : existing message box

Fig : less than or equal to 20 changes confirmation box

Fig : more than 20 changes confirmation box

Fig : no param change saved box

Fig : one or more param change saved box

[prathamEndu]

@robertlong13 and @EosBandi
Can you please review the changes if you get some time.

[EosBandi]

Line 256, there is a generic are you sure question, then you asked again... I think the first question should be removed.

MainV2.comPort.MAV.param always contains the cached param list, so it is not possible that a param to change not in the array, so error counting and display "Read faliure" message is unneccessary.

[prathamEndu]

Said changes are done :

1. Redundant question removed
2. unnecessary error handling removed

[prathamEndu]

@EosBandi, do you notice any additional changes that might be necessary?

[prathamEndu]

@robertlong13 can you give this a look when free?

[robertlong13]

Overall I like the idea, but there are some things I don't like:

1. I don't like formatting this as text in a messagebox. Comparison of the before/after should really be something like a gridview, though that'd be a lot more work for you.
2. Even 20 might make the window a bit too big on Android. Would need to double check that.

I'd prefer dropping the compare from this entirely and have the popup just tell you how many parameters got changed, but I don't hate it as-is (after fixing the requested changes I made in my other comments). Michael is the authority on this though.

[prathamEndu]

1. I don't like formatting this as text in a messagebox. Comparison of the before/after should really be something like a gridview, though that'd be a lot more work for you.

I would be very interested in trying that if you can direct me to an example of how to implement it, or any direction for where to look.

2. Even 20 might make the window a bit too big on Android. Would need to double check that.

I’m unsure how to test this on Android or even how to build and test it for that platform. Could you provide guidance on this?

I'd prefer dropping the compare from this entirely and have the popup just tell you how many parameters got changed, but I don't hate it as-is (after fixing the requested changes I made in my other comments). Michael is the authority on this though.

I’d prefer not to drop the compare functionality entirely, as the main goal of this issue is to provide visibility into what’s being changed right before committing. The requested changes have been addressed.

Also, who is Michael, and how do I get this reviewed by him?

[robertlong13]

I would be very interested in trying that if you can direct me to an example of how to implement it, or any direction for where to look.

You'd have to make a new form in designer. Could look at paramcompare.cs as an example, but it's not really necessary.

I’m unsure how to test this on Android or even how to build and test it for that platform. Could you provide guidance on this?

Normally I grab it from the automatic builds, but it looks like they need approval to run. You can try to run them on your own fork.

Also, who is Michael, and how do I get this reviewed by him?

He's the maintainer of Mission Planner, and the only one who can actually approve anything. My review and comments are merely symbolic.

[prathamEndu]

@meee1
When you have some free time, could you please review this and suggest any necessary changes?

[rmackay9]

It's good to see new contributors to MP!

I think we should add a new "never show again" box. Seeing the new box every time I change a parameter is a bit annoying actually and just slows me down.

[rmackay9]

.. after playing with it a bit more though, my opinion has changed a bit. I still think we should add the "don't show again" checkbox but in general it is actually quite helpful to see the pop-up with all the params that have been changed. It makes it easy to double check that you haven't made any mistakes

[robertlong13]

It can't slow you down, the new pop-up replaces the old "Are you Sure?" pop-up, and we didn't have a "don't show again" for that one.

[rmackay9]

Hi @robertlong13,

ok thanks. I thought there was a way to stop the "Are you Sure?" from coming up but maybe I'm misremembering. I thought there was a "don't show again" but maybe it didn't always work. Anyway, moving on, txs!

[robertlong13]

I thought there was a way to stop the "Are you Sure?" from coming up but maybe I'm misremembering. I thought there was a "don't show again" but maybe it didn't always work. Anyway, moving on, txs!

I'm terribly sorry, you're absolutely right. It was hidable. I was wrong.

We really should add that back.

[prathamEndu]

.. after playing with it a bit more though, my opinion has changed a bit. I still think we should add the "don't show again" checkbox but in general it is actually quite helpful to see the pop-up with all the params that have been changed. It makes it easy to double check that you haven't made any mistakes

Firstly, I am happy that you find it useful, I made it after realizing the need for such feature myself.

If I am right, you want to have a option to hide the "Confirmation box" and not the "Saved message box".
so user who doesn't want that popup can hide it. but will still see the final parameter change "Saved message box".

sounds good, just 2 question:

1. any example where such thing exist within mission planner, I couldn't see this hidable box feature right now, or maybe i missed something.
2. also, how will he re-enable it if he wants to? and where should that option be placed? In "planner page", or right below the 2 existing options for modifiable and non default in the "parameter page".

[robertlong13]

1. any example where such thing exist within mission planner, I couldn't see this hidable box feature right now, or maybe i missed something.

The "Are you Sure" box you replaced is an example:

Common.MessageShowAgain("Write Raw Params", "Are you Sure?")

It automatically generates the checkbox and saves the settings. It uses the title of the box to create the name of the setting, so you want to make sure to pick a unique title (which you have, the title you have on your message box will work great).

2. also, how will he re-enable it if he wants to? and where should that option be placed? In "planner page", or right below the 2 existing options for modifiable and non default in the "parameter page".

They'd have to reset their settings to default (delete their config.xml) or open the config.xml and look for the ShowAgain setting that matches that window. We don't really have a general way to turn them back on, but that's okay I think.

You could add a separate setting for it, but the easiest is just to use that MessageShowAgain form.