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

User memory traj buffer #130

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

Conversation

LeandroMartinsdS
Copy link
Contributor

@LeandroMartinsdS LeandroMartinsdS commented Nov 14, 2024

Currently PowerPMAC trajectory is using global variables (P-variables), that has a restricted number of elements and, in Diamond context, is shared by other applications - e.g. PLCs.

On PowerBrick, by default 1 MB allocated for User Shared-Memory (USHM), being possible to extend that up to 64MB.

@LeandroMartinsdS LeandroMartinsdS force-pushed the userMemoryTrajBuffer branch 2 times, most recently from 4d02341 to fe2d34f Compare November 14, 2024 13:00
@LeandroMartinsdS LeandroMartinsdS marked this pull request as draft November 14, 2024 13:01
@LeandroMartinsdS LeandroMartinsdS marked this pull request as ready for review December 2, 2024 13:59
@JamesOHeaDLS
Copy link

Looks good overall. I just suggest a few extra comments to explain the data structure. Thanks

@@ -4652,7 +4696,7 @@ asynStatus pmacController::tScanCalculateVelocityArray(double *positions, double
int prevBuffLastVelMode = -1;
double prevBuffPosition = 0;
double prevBuffVelocity = 0;
int prevBuffTime = 0;
double prevBuffTime = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for myself: not related with this PR, and can't remember why I have changed. Possibly will revert this.

@@ -194,7 +189,31 @@ Return
// *************************************************************************************************

N103:
CalculatedBase = CurrentBufferAdr + CurrentIndex
CalculatedBase = CurrentBufferAdr/4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(note for myself)
Substitute for
CalculatedBase = CurrentBufferAdr/_SIZEOF_INTEGER_

@@ -189,30 +189,30 @@ Return
// *************************************************************************************************

N103:
CalculatedBase = CurrentBufferAdr + CurrentIndex*4
CalculatedBase = CurrentBufferAdr/4

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment explaining the divide by 4 would be useful here

X_Vel_Idx = W_Vel_Idx + BufferLength
Y_Vel_Idx = X_Vel_Idx + BufferLength
Z_Vel_Idx = Y_Vel_Idx + BufferLength
A_Idx = CalculatedBase/2 + BuffLen + CurrentIndex

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, a comment explaining the divide by 2 would be useful here

#define BuffAddr 4000
#define BuffAddr 4000 // >8 to keep first byte unused - $8 used by AxesParser
#define BuffLen 2000 // Length of buffers
#define DoubleBuffLen 4000 // BuffAddr + DoubleBuffLen * (2*sizeof(int)+18*sizeof(double)) <= User Buffer size

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe DoubleBuffLen should be defined as 2 * BuffLen

void pmacHardwareTurbo::startTrajectoryTimePointsCmd(char *user_cmd, char *time_cmd,
int addr) {
int addr, int) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an int data type without variable name?

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