Default boot partition specified in recovery.cmdline never gets used

[lurch]

While investigating #150 I found that if you have more than one OS installed, and you haven't yet booted any of the OSes (i.e. the countdown-timer doesn't activate on the OS-boot menu) then if you power-off, and set a partition=<number> in recovery.cmdline as instructed to change the default boot partition, then when you boot up the Pi it doesn't select that partition as the default like you'd expect it to.

Did some digging, and eventually got to the bottom of the problem: https://github.com/raspberrypi/noobs/blob/master/recovery/bootselectiondialog.cpp#L94
and https://github.com/raspberrypi/noobs/blob/master/recovery/bootselectiondialog.cpp#L96 and https://github.com/raspberrypi/noobs/blob/master/recovery/mainwindow.cpp#L461

The problem is that as soon as NOOBS has finished installing all OSes, it sets "default_partition_to_boot" to 800 straight away, which means settings.value("default_partition_to_boot", defaultPartition).toInt() will also always return 800 (if "default_partition_to_boot" hasn't been set to some other value yet by actually booting an OS), and thus the defaultPartition value never actually gets picked up and used. The defaultPartition value itself also gets initialised to 800 here https://github.com/raspberrypi/noobs/blob/master/recovery/main.cpp#L92

Also, https://github.com/raspberrypi/noobs/blob/master/recovery/main.cpp#L43 seems to be assuming that the presence of a /dev/mmcblk0p7 partition indicates that we have more than one OS installed (based on the assumption that each OS uses 2 partitions). Of course this isn't strictly true - we might have a single OS using 3 partitions, or we might have 2 OSes each only using a single partition (if they're each booting from an initrd).

[Ruffio]

@lurch is this still an issue?

[procount]

I'm just looking at this because it is similar behaviour to the display= parameter. Both of these just set the DEFAULT value if a value doesn't already exist in the settings file. So they are only ever used if they are set on first boot of NOOBS.

I'm thinking that it would be more useful if both of these cmdline parameters allowed the user to OVERRIDE any value in the the settings. Otherwise, once their values are set in the settings file, any further parameters are no longer used. It would then be a way to change the default values which would subsequently be stored in the settings file.

[lurch]

But that sounds like a backwards-incompatible change, so it would probably need some careful thought / documentation? i.e. users might get confused why the changes they're making in the NOOBS GUI aren't getting "saved", not realising that they're still being overridden by the values set in recovery.cmdline? 🤷‍♂️
Remember that one of the goals of NOOBS is to never touch the first partition, so we can't have NOOBS itself removing options from recovery.cmdline (with the obvious caveat of the runinstaller option).

[procount]

I wasn't thinking of letting NOOBS/PINN change/revert the cmdline, but rather inform the user they must remove it themselves. Currently, these parameters only seem to be effective if you set them before you first run NOOBS. I don't think this is explained well, which is why there are these 2 outstanding issues. I just think they don't do what users expect them to do, and once NOOBS has been run once, they do nothing at all.

EDIT: I suppose an alternative would be to add 2 additional parameters that override the settings, thus avoiding the backward compatibility issue?

[lurch]

For something aimed at newbies, "inform the user they must remove it themselves" doesn't sound like very user-friendly behaviour for NOOBS. (but of course it might fit in well with the more advanced functionality offered by PINN)

Re-reading the original comment in this issue (oh my, nearly 5 years ago!) I think my intention was that if a "default boot partition" hadn't explicitly been set yet (i.e. if the user had installed several OSes but hadn't actually booted any of them yet) then it would ignore the "800" value and still pick up the value from recovery.cmdline ? 🤷‍♂️
But as this is such a low-importance issue I never bothered doing any work on it. Seems unlikely that a user would use NOOBS to install several OSes, and then immediately power-off without actually booting any of them.