Further improvement on the number of configurable devices

[GioCC]

I think this is the right moment to introduce a (semi-)new topic in the wake of @elral's last changes (particularly issues #127 and #148), that have built an excellent foundation for a further step in the direction of making the firmware more efficient.

I'm nearly finished with some modifications that will allow the full "pseudo-dynamic" allocation of devices. This basically means several improvements:

• there are no longer fixed numbers of devices of each type (no more MAX_LEDSEGMENTS, MAX_MFSERVOS, MAX_INPUT_SHIFTERS...).
  As long as available space is sufficient, a user could define any number of LED displays or Servos or encoders... in any desired combination (within the hardware pin limits of course).
• Input and output devices are handled in a fully uniform way (technically, by superclassing them with an "MFdevice" class);
• Beside the fact that it's more efficiently used, there could also be an increase in available RAM and or code space.

About the first point, a reminder note just for the sake of completeness: as already discussed elsewhere, a more efficient use of the RAM "only" allows to optimize the number of devices that can be configured. A similar improvement regarding the set of device types would be interesting, but this is related to the flash memory usage, which implies a change at the level of the compilation process.

The firmware remains fully compatible with the Connector, although it could benefit from some modifications in the latter.
The main one could be that a "memory full" condition could (in practice) only be detected once the configuration is uploaded; so the user has to be properly notified, possibly adding some specific information that helps him in solving the issue.

As of now, I am close to completing the code; I have no indication yet about the possible gains (or losses?) regarding the last point above, but I'm planning to verify them with the tests that will immediately follow completion.

One thing is for sure: the extent of the changes (particularly in combination with the ones introduced by @elral) will require a good round of testing from anyone that's willing to contribute.

In a few days I will send the link to my git branch so anyone interested can have a look. (@DocMoebiuz , @neilenns, ...)

I'm obviously looking forward to your feedback and comments.

[GioCC]

This is an update on the progress from the initial idea above.

Starting point:

Since the substantial PR #147 by @elral, I wanted to push his modifications a little bit further to completion.
Thanks to his work, most device objects are now no longer allocated statically, but placed in a common buffer, so (mostly) space is only assigned when required.
This implementation however has still some points that can be improved.

TL;DR: Not all (parts of the) objects are allocated dynamically, plus some space is occupied by the index vectors (e.g. encoders[], ledSegments[]…)

Technical details (you can skip this section if not interested):

• For inputs, device objects themselves are fully allocated in the common buffer, but index vectors with object pointers still exist.
  These vectors must be sized to contain each the maximum allowable number of each device type.
• For outputs, devices are made by the MF device object plus the actual "worker" object (e.g.: Steppers are made by one MFStepper + one AccelStepper; LED displays are made by an MFsegments plus a LedControl etc.).
  The index vectors for output devices contain the whole MF device object (obviously larger than the pointers alone), while merely the "worker" object is placed in the common buffer.

The improvement consists in removing index vectors altogether (using different object management structures) and placing all data in the common buffer.
The elimination of index vectors allows to use just the exact RAM amount required, without the need for extra provisioning (either for pointers or whole objects).
A related perk is that there is no longer a fixed limit for any kind of devices.
This solution allows a non negligible gain in RAM space (both because of higher efficiency and elimination of unnecessary provisioning), without any penalty in Flash occupation. (Gains have been quantified, but it’s a little hard to present them with a single global figure).

A sideline story:

In the meantime, following some comments regarding PR #152, I was trying to help by "de-composing" the individual changes included in the PR with the aim of putting them in the form of individual issues/PR pairs to speed up acceptance.

I started a branch with individual commits targeting following points:

1. Move files to new locations
2. Mobiflight.cpp split to config.* / commandmessenger.* / inputHub.* / outputHub.* (see below)
3. Poll time schedules moved to main
4. DEBUG constant renamed to DEBUG2MSG
5. MFEEPROM.init() moved into the constructor
6. Removed obsolete Arduino includes for Arduino version < 1.00 (not included in Elral/issue148 #152)
7. Replaced "include guards" in .h files with #pragma directives

One thing I did not change yet was the splitting of client functions that use MFxxx device objects (e.g. AddDevice()) and input event handlers / output commands to individual files (e.g. Encoder.* + MFEncoder.*), because that would have been confusing for the sources I was working on.
Instead, I gathered all these functions in two files (pairs), InputHub.* and OutputHub.*. These have become more manageable anyway because the functions contained have become smaller, so I saw no real need to split them anymore.

Putting it all together:

After some chats with @elral, during which I explained the whole situation to him, he kindly agreed that we could possibly proceed this way (pending his final approval):

• replace his PR Elral/issue148 #152 with a PR from my "detailed" branch;
• waive the file split for device client functions;
• on that base, add a new PR for my final modifications described above.

As soon as the final details are reviewed by @elral and me, I will immediately make the links to the repository branches available for evaluation.

Final request:

@DocMoebiuz @neilenns I would appreciate if you could briefly read the above and let me know if we can proceed this way or there are any suggestions or comments.
Thanks

[DocMoebiuz]

first of all thanks to everyone for your efforts and passion that you are putting into it.

We have to be careful to not end up having too many moving pieces. I am friend of simplicity and taking smaller steps, one at a time.

We have been living with the current firmware for so long. We have made a lot of memory optimizations. We don't have to have all optimizations in one step.

Let's finish merging the work by @elral:

• Improved dynamic memory allocation
• Splitting up mobiflight cpp and bundle code and files by dev type as much as possible
  Once we have done this we have a new baseline for further improvements.

If we don't finish these changes first we will have a lot of headache with merging changes back and forth into branches.

It's great to see that there are so many ideas which we can tackle then easier.

A general thought and decision that i have taken is that i want to focus on a better and easier way allowing people to bring their own (exotic) devices to Mobiflight.

This means that we have to be careful to not make the core too complicated to understand.

The general idea is to not include all these different devices into the official MobiFlight Core Firmware but focus on simple yet flexible interfaces for adding individual stuff if a user wants to. We can then allow users to build their own firmware if they need it. things like, I would like TMxxxx instead of Max7219 will then be possible.

I am thinking of creating a place where we can collect community contributions, e.g. maintaining a list in a wiki/github.

I strongly believe that decoupling the community devices from the MobiFlight Core Firmware will increase the speed with which we can support a broader range of ICs. Based on the community feedback and interest we can then still decide if a device will eventually make it i to the MF Core Firmware. It's kind of like an "Incubator".

With this idea in mind, I believe the way how Ralf wanted to split and bundle the files was a good fit.

I will finish the current MF Connector Release and then I will be able to focus more on the firmware. It's just a matter of a couple of days until 9.3 will be released.

Feedback welcome! You're awesome!

[GioCC]

Hey, thanks for responding so quickly!

Let's finish merging the work by @elral:
[...]
Once we have done this we have a new baseline for further improvements.

That's exactly what I asked for in my request: to not let it float for too long, so we can be sure of the base we are working on.
Small and safe steps are only possible if the previous steps are already laid.

If we don't finish these changes first we will have a lot of headache with merging changes back and forth into branches.

Fully agreed... and that is exactly my main concern. That's why I tried to prepare a set of files that could maybe bring to a quicker acceptance.

A general thought and decision that i have taken is that i want to focus on a better and easier way allowing people to bring their own (exotic) devices to Mobiflight.
This means that we have to be careful to not make the core too complicated to understand.

I don't really see this being currently an issue... at least, I don't see anything that might be making this aspect worse, but rather the opposite.

The general idea is to not include all these different devices into the official MobiFlight Core Firmware but focus on simple yet flexible interfaces for adding individual stuff if a user wants to. We can then allow users to build their own firmware if they need it. things like, I would like TMxxxx instead of Max7219 will then be possible.

This is a good idea, and I think the firmware is remarkably modular in this respect, as firmware goes.
Still, I think it is most applicable to really "exotic" or custom devices. There are still a few devices of current use - or used in certain configurations - that are eligible of being included in the core FW (e.g.: MCP23x17, the TM1637 you mention); but as things stand, this would imply having the possibility of conditional FW compilation, which - regardless of further device additions - is a topic that'll be worth considering.
And another issue that's probably harder to tackle is that such a feature would involve the connector in a less trivial way, because I expect a need for more diverse interfaces on that side.

I would also mention that, for this goal, just a modicum of technical documentation would probably be useful. Not end-user tutorials (which are also needed, and in larger quantity) but some basic information, like e.g. the documentation of the serial protocol or a brief overview of how the system works and how the software workflow is structured.

I am thinking of creating a place where we can collect community contributions, e.g. maintaining a list in a wiki/github.
I strongly believe that decoupling the community devices from the MobiFlight Core Firmware will increase the speed with which we can support a broader range of ICs. Based on the community feedback and interest we can then still decide if a device will eventually make it into the MF Core Firmware. It's kind of like an "Incubator".

I'm not sure I understand what it is that you have in mind practically.
To begin with, the firmware has undergone several optimizations and improvements regarding mainly the memory requirements, for good reasons. Thinking of integrating contributions for external devices in the core is, in my opinion, a bit too stretched, since it requires some attention and knowledge not to break firmware operation... I don't really see the chance of effectively achieve any real "decoupling".
Unless it all sums up to contributions as already done now, but then I don't quite see the point.

On the other hand, allowing users to write their own firmware in order to support their own device types (or any other feature, for that matters), using a "base" version of the core firmware as a sort of sample / framework, that could be a very interesting provision.

With this idea in mind, I believe the way how Ralf wanted to split and bundle the files was a good fit.

As I never held back, I think the splitting of device files goes against the goal of achieving more clarity and a better oversight when working with the source. But the main reason I asked Ralf if he would be inclined to dispense with it was because it would have complicated the introduction of the feature in subject. That said, it's something I can surely live with.

I will finish the current MF Connector Release and then I will be able to focus more on the firmware. It's just a matter of a couple of days until 9.3 will be released.

I would really appreciate it; most of what I'm working on is quickly grinding to a halt. For one, my PR for the multiplexers (which also involves the Connector) is dragging along and it's costing me a lot of effort for just keeping the pace of all new merges on the main branch.

[neilenns]

Just one general thought: any change to the firmware that gets rid of the per-device limit will be the first time (at least since I've been involved in MobiFlight) where firmware changes will have a corresponding change on the desktop app. The desktop app also maintains the max of each type of device (in the board.json files) and verifies that when adding modules.

We'd need to think through whether there is a hard dependency on firmware version vs. desktop app version should a change like this be done. What happens if the desktop also gets rid of the check but then connects to devices without a firmware with the new change? What about the other way around (new firmware, older desktop?).

[GioCC]

Actually, almost nothing would change. As it is, the hardcoded device limit is oversized (you can't define the max number of all devices: you simply can't define more than the max for each device).
If too many devices are defined, the firmware returns a "memory full" message, that the user has to deal with (I'm referring to @elral's modification, where things already work this way).
The current connector is compatible with the new firmware (possibly not all arduino resources will be exploited); as for the other way around, I don't see a problem, since the Connector does detect an older firmware version and ask to upgrade (even though they would not be strictly incompatible, but the per-device limit would be no longer enforced by the connector prior to upload).

[GioCC]

BTW, that's another advantage: no need to keep limit values synced between the two parts, with potential mismatches.

[GioCC]

If anybody is interested in looking at the actual code, as it would appear at the end of the changes, here is the link.
I will make an effort to see if splitting device files again (i.e. "exploding" inputHub.* and outputHub.*) still makes it manageable, otherwise it will be probably worth just to stash it and maybe wait to see it the code will still be usable sometime in the future.

In any case, I'd still like to have a feedback on whether the itemized version of @elral 's PR can still be of interest (integrated with the last changes I left pending).

[GioCC]

In any case, I'd still like to have a feedback on whether the itemized version of @elral 's PR can still be of interest (integrated with the last changes I left pending).

OK, I see #152 has just been merged, therefore I guess the answer is no.