DiamondLightSource / pmac

Driver for the Delta Tau PMAC motion controller family.
Apache License 2.0
25 stars 17 forks source link

PowerPMAC driver suggested improvements #108

Open ajgdls opened 1 year ago

ajgdls commented 1 year ago

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.

LeandroMartinsdS commented 3 months ago

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.