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

PowerPMAC driver suggested improvements #108

Open
wants to merge 2 commits into
base: dls-master
Choose a base branch
from

Conversation

ajgdls
Copy link
Collaborator

@ajgdls ajgdls commented Oct 11, 2023

This PR is suggested updates to the PowerPMAC driver to remove magic numbers, and to improve the memory allocation for the large character arrays.
The arrays have been assigned memory via malloc rather than allocated on the stack, to allow larger arrays, and to avoid the constant reallocation of the same arrays every time a write/read is called. The low level driver is already NOT thread safe and so allocating the memory in this way does not alter this, but the mechanism can be discussed.

I recommend testing these changes extensively (with the largest use case we have) to ensure robustness.

Updated MAX_CHAR_LENGTH to 7168.
Added DOUBLE_MAX_LENGTH definition.
@LeandroMartinsdS
Copy link
Contributor

To keep track of discussion made via email, regarding PPMAC_MAX_CHAR_LENGTH

On the PowerBrick (firmware 2.7.1), I have made a quick test doing an ssh and trying to send as many characters as possible into P-variables.

Something like:
P(0)=1000000000,1000000001,1000000002...100000000323,...
and so on

When I checked the P(323), I only got
P323=1
instead of
P323=100000000323

From the P324, all have the value 0.

Therefore the number of characters properly assigned was 4095 and no error was raised, different from what the release notes specify.
It was a very simple test, so I wouldn't be too confident about its result, but it might give us some idea about the limitation.

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.

3 participants