Skip to content

Modifications to get rid of device index arrays#175

Closed
GioCC wants to merge 12 commits intoMobiFlight:mainfrom
GioCC:No_device_arrays_PR
Closed

Modifications to get rid of device index arrays#175
GioCC wants to merge 12 commits intoMobiFlight:mainfrom
GioCC:No_device_arrays_PR

Conversation

@GioCC
Copy link
Contributor

@GioCC GioCC commented Apr 23, 2022

Description of changes

Addresses issue #174 (which in turn refers to discussion #154).
The "index" arrays for each device type are suppressed, and devices are allocated completely in the already existing object buffer; this way, there is no longer a limit to the number of devices of a given type that can be allocated (e.g.: one can allocate exclusively shift registers, as many as the avaliable space allows). The max amounts need no longer to be defined.

Notes

  • Modifications are broad (also because they involve all devices), but mostly fairly repetitive.
    Also, the refactoring contributes IMHO to a certain "modularization" of the structure of device-related code.
  • The current allocateMem class is replaced by a slightly more complex stowManager, which has the same basic purpose but with more extended functions.
  • There is a (slight) gain in RAM, although this is not the actual goal.

NOTE: This is a draft PR; the code, albeit complete, still requires testing and cleanup (and obviously also review).
If interested, please don't forget to have a look at the comments in the IODevice class, which is pivotal.

I will greatly appreciate any checking effort and suggestions.

@GioCC GioCC force-pushed the No_device_arrays_PR branch from 3d1bf96 to cabaff5 Compare April 23, 2022 19:02
@github-actions
Copy link

Firmware for this pull request:
Firmware.zip

@github-actions
Copy link

Firmware for this pull request:
Firmware.zip

@github-actions
Copy link

github-actions bot commented May 9, 2022

Firmware for this pull request:
Firmware.zip

@elral
Copy link
Collaborator

elral commented May 11, 2022

Additional to my comment above, changes in config.cpp are required. Can't do it in the code, as this file is not changed within this Pull Request. Revert Back from pullrequest #173 is not merged here:

bool readRecordTailFromEEPROM(char **dest = NULL, char *cap = (char *)0xFFFF)
{
    char temp = 0;
    do {
        temp = MFeeprom.read_char(); // read the first character
        if (dest) {
            *((*dest)++) = temp;
            // TODO: this function should be buffer-agnostic -
            //  the specific constant for buffer size should not be hardcoded!
            if (*dest >= cap) break; // nameBuffer full: stop copying
        }
    } while (temp != ':'); // reads until limiter ':' and locates the next free buffer position
    if (dest) {
-------------------------------
//        *((*dest)--) = 0x00; // replace ':' by NULL, terminates the string
        *((*dest)-1) = 0x00; // replace ':' by NULL, terminates the string
    }
//    return (*dest >= cap);
    return (*dest <= cap);
------------------------------
}

I am not completly sure about *((*dest)-1) = 0x00; // replace ':' by NULL, terminates the string

Nevertheless my mega2560 crashes even with this changes (I tested them before reverting the main branch and they were OK). W/o them the config gets not loaded and the mega2560 doesn't crash.

@elral
Copy link
Collaborator

elral commented May 11, 2022

Hmhm, tested it again. Now the mega does not crash!?! Please don't ask me what I did wrong before. Sorry.
But it seems that the config get's still not loaded. No input and output device is working...

@GioCC
Copy link
Contributor Author

GioCC commented May 11, 2022

Hmhm, tested it again. Now the mega does not crash!?! Please don't ask me what I did wrong before. Sorry. But it seems that the config get's still not loaded. No input and output device is working...

Ok, I think I'll have to take a step back and review it again myself first. I have been trying to follow late changes (basically PRs from #171 on) and it's getting a little confusing; I'd like to have a look to last-minute changes because there's definitely something that doesn't add up. Thanks for your effort so far, I will get back to you all once I figure out where it's at.

BTW, both the changes you pointed out are clearly correct!

@GioCC GioCC force-pushed the No_device_arrays_PR branch from 12be5fb to 0dbea35 Compare May 11, 2022 07:51
@github-actions
Copy link

Firmware for this pull request:
Firmware.zip

@elral
Copy link
Collaborator

elral commented May 11, 2022

What I found so far, Button::Add(params[0], nameBufPtr); leads to a crash.
With the above mentioned changes the readConfig() will be finished (w/o button configured), but then it seems to crash.

edit:/ checking Button::Add() gets out of my knowledge....

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Firmware for this pull request:
Firmware.zip

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Firmware for this pull request:
Firmware.zip

@GioCC GioCC closed this Oct 4, 2022
@GioCC GioCC deleted the No_device_arrays_PR branch January 29, 2023 19:01
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

Successfully merging this pull request may close these issues.

2 participants