-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Expand hardware drivers to 64 bits (address issue #2331) #3932
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
|
|
||
| #include <rtapi.h> // RTAPI realtime OS API | ||
| #include <rtapi_app.h> // RTAPI realtime module decls | ||
| #include <rtapi_stdint.h> // portable ints | ||
| #include <hal.h> // HAL public API decls | ||
| #include "gm.h" // Hardware dependent defines | ||
| #include <rtapi_math.h> | ||
|
|
@@ -16,9 +17,11 @@ typedef struct { //encoder_t | |
| // Pins | ||
| hal_bit_t *reset; | ||
| hal_s32_t *counts; | ||
| hal_s64_t *counts64; | ||
| hal_float_t *position; | ||
| hal_float_t *velocity; | ||
| hal_s32_t *rawcounts; | ||
| hal_s64_t *rawcounts64; | ||
| hal_bit_t *index_enable; | ||
|
|
||
| // Parameters | ||
|
|
@@ -30,11 +33,11 @@ typedef struct { //encoder_t | |
| hal_float_t min_speed_estimate; | ||
|
|
||
| // Private data | ||
| hal_s32_t raw_offset; | ||
| hal_s32_t index_offset; | ||
| hal_s32_t last_index_latch; | ||
| rtapi_s64 index_offset; | ||
| rtapi_s64 last_index_latch; | ||
| hal_bit_t first_index; | ||
| hal_bit_t module_exist; | ||
| rtapi_s32 old_counts; | ||
| } encoder_t; | ||
|
|
||
| typedef struct { //switches_t | ||
|
|
@@ -239,27 +242,27 @@ typedef struct { //stepgen_t | |
| hal_float_t *position_cmd; | ||
| hal_float_t *velocity_cmd; | ||
| hal_float_t *position_fb; | ||
| hal_s32_t *count_fb; | ||
| hal_s64_t *count_fb; | ||
| hal_bit_t *enable; | ||
|
|
||
| // Parameters | ||
| hal_u32_t step_type; //0: StepDir, 1: UpDown, 2: Quadrature | ||
| hal_bit_t control_type; //0: position, 1: velocity | ||
| hal_bit_t control_type; //0: position, 1: velocity | ||
| hal_u32_t steplen; | ||
| hal_u32_t stepspace; | ||
| hal_u32_t dirdelay; | ||
| hal_float_t maxaccel; | ||
| hal_float_t maxvel; | ||
| hal_bit_t polarity_A; | ||
| hal_bit_t polarity_B; | ||
| hal_bit_t polarity_B; | ||
| hal_float_t position_scale; | ||
|
|
||
| //Saved Parameters | ||
| hal_u32_t curr_steplen; | ||
| hal_u32_t curr_stepspace; | ||
| hal_u32_t curr_dirdelay; | ||
| hal_float_t curr_maxaccel; | ||
| hal_float_t curr_maxvel; | ||
| hal_float_t curr_maxvel; | ||
| hal_float_t curr_position_scale; | ||
|
|
||
| // Private data | ||
|
|
@@ -623,9 +626,11 @@ ExportEncoder(void *arg, int comp_id, int version) | |
| //Export Pins | ||
| if(error == 0) error = hal_pin_bit_newf(HAL_IN, &(device->encoder[i].reset), comp_id, "gm.%1d.encoder.%1d.reset", boardId, i); | ||
| if(error == 0) error = hal_pin_s32_newf(HAL_OUT, &(device->encoder[i].counts), comp_id, "gm.%1d.encoder.%1d.counts", boardId, i); | ||
| if(error == 0) error = hal_pin_s64_newf(HAL_OUT, &(device->encoder[i].counts64), comp_id, "gm.%1d.encoder.%1d.counts64", boardId, i); | ||
| if(error == 0) error = hal_pin_float_newf(HAL_OUT, &(device->encoder[i].position), comp_id, "gm.%1d.encoder.%1d.position", boardId, i); | ||
| if(error == 0) error = hal_pin_float_newf(HAL_OUT, &(device->encoder[i].velocity), comp_id, "gm.%1d.encoder.%1d.velocity", boardId, i); | ||
| if(error == 0) error = hal_pin_s32_newf(HAL_OUT, &(device->encoder[i].rawcounts), comp_id, "gm.%1d.encoder.%1d.rawcounts", boardId, i); | ||
| if(error == 0) error = hal_pin_s64_newf(HAL_OUT, &(device->encoder[i].rawcounts64), comp_id, "gm.%1d.encoder.%1d.rawcounts", boardId, i); | ||
| if(error == 0) error = hal_pin_bit_newf(HAL_IO, &(device->encoder[i].index_enable), comp_id, "gm.%1d.encoder.%1d.index-enable", boardId, i); | ||
|
|
||
| //Export Parameters | ||
|
|
@@ -637,7 +642,7 @@ ExportEncoder(void *arg, int comp_id, int version) | |
| if(error == 0) error = hal_param_float_newf(HAL_RW, &(device->encoder[i].min_speed_estimate), comp_id, "gm.%1d.encoder.%1d.min-speed-estimate", boardId, i); | ||
|
|
||
| //Init parameters | ||
| device->encoder[i].raw_offset = pCard->ENC_counter[i]; | ||
| device->encoder[i].old_counts = pCard->ENC_counter[i]; | ||
| device->encoder[i].index_offset = 0; | ||
| device->encoder[i].last_index_latch = pCard->ENC_index_latch[i]; | ||
| device->encoder[i].first_index = 1; | ||
|
|
@@ -679,7 +684,7 @@ ExportStepgen(void *arg, int comp_id, int version) | |
| if(error == 0) error = hal_pin_float_newf(HAL_IN, &(device->stepgen[i].position_cmd), comp_id, "gm.%1d.stepgen.%1d.position-cmd", boardId, i); | ||
| if(error == 0) error = hal_pin_float_newf(HAL_OUT, &(device->stepgen[i].position_fb), comp_id, "gm.%1d.stepgen.%1d.position-fb", boardId, i); | ||
| if(error == 0) error = hal_pin_float_newf(HAL_IN, &(device->stepgen[i].velocity_cmd), comp_id, "gm.%1d.stepgen.%1d.velocity-cmd", boardId, i); | ||
| if(error == 0) error = hal_pin_s32_newf(HAL_OUT, &(device->stepgen[i].count_fb), comp_id, "gm.%1d.stepgen.%1d.count-fb", boardId, i); | ||
| if(error == 0) error = hal_pin_s64_newf(HAL_OUT, &(device->stepgen[i].count_fb), comp_id, "gm.%1d.stepgen.%1d.count-fb", boardId, i); | ||
| if(error == 0) error = hal_pin_bit_newf(HAL_IN, &(device->stepgen[i].enable), comp_id, "gm.%1d.stepgen.%1d.enable", boardId, i); | ||
|
|
||
| //Export Parameters. | ||
|
|
@@ -1472,7 +1477,7 @@ encoder(void *arg, long period) | |
| card *pCard = device->pCard; | ||
|
|
||
| int i; | ||
| hal_s32_t temp1 = 0, temp2; | ||
| rtapi_s32 temp1 = 0, temp2, delta_counts; | ||
| hal_float_t vel; | ||
|
|
||
| //Update parameters | ||
|
|
@@ -1529,12 +1534,15 @@ encoder(void *arg, long period) | |
| } | ||
| } | ||
| device->encoder[i].last_index_latch = temp2; | ||
|
|
||
| *(device->encoder[i].rawcounts) = temp1 - device->encoder[i].raw_offset; | ||
| delta_counts = temp1 - device->encoder[i].old_counts; | ||
| device->encoder[i].old_counts = temp1; | ||
| *(device->encoder[i].rawcounts) += delta_counts; | ||
| *(device->encoder[i].rawcounts64) += delta_counts; | ||
|
Comment on lines
+1537
to
+1540
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks fine. You do: s32_deltacount = s32_counter - s32_saved_counter;
s32_saved_counter = s32_counter;
s64 += s32_deltacount;I assume that the s32_counter is two-s complement. However, it should also work for an unsigned counter because the deltacount should always be result in correct two's complement.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only assumption is that you cannot wrap a 32-bit counter in one servo period. But I guess we should be save there?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so. The Mesa counters max out at 20MHz, the internet suggests that 500MHz is possible.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At 500MHz you have a comfortable 8.5 seconds before you wrap :-) |
||
| *(device->encoder[i].counts) = *(device->encoder[i].rawcounts) - device->encoder[i].index_offset; | ||
| *(device->encoder[i].counts64) = *(device->encoder[i].rawcounts64) - device->encoder[i].index_offset; | ||
|
|
||
| if((device->encoder[i].position_scale < 0.000001) && (device->encoder[i].position_scale > -0.000001)) device->encoder[i].position_scale = 1; //Don't like to divide by 0 | ||
| *(device->encoder[i].position) = (hal_float_t) *(device->encoder[i].counts) / device->encoder[i].position_scale; | ||
| *(device->encoder[i].position) = (hal_float_t) *(device->encoder[i].counts64) / device->encoder[i].position_scale; | ||
|
|
||
| vel = (hal_float_t) pCard->ENC_period[i]; | ||
| if(vel == 0) vel = 1; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,7 @@ | |
| #include <rtapi_io.h> /* kmalloc() */ | ||
| #include <rtapi.h> /* RTAPI realtime OS API */ | ||
| #include <rtapi_app.h> /* RTAPI realtime module decls */ | ||
| #include <rtapi_stdint.h> // portable ints | ||
| #include <hal.h> /* HAL public API decls */ | ||
| #include <rtapi_parport.h> | ||
|
|
||
|
|
@@ -293,12 +294,13 @@ typedef union { | |
| typedef struct { | ||
| hal_float_t *position; /* output: scaled position pointer */ | ||
| hal_s32_t *count; /* output: unscaled encoder counts */ | ||
| hal_s64_t *count64; /* 64 bit version to avoid wrapping */ | ||
| hal_s32_t *delta; /* output: delta counts since last read */ | ||
| hal_s32_t prevdir; /* local: previous direction */ | ||
| hal_float_t scale; /* parameter: scale factor */ | ||
| hal_bit_t *index; /* output: index flag */ | ||
| hal_bit_t *index_enable; /* enable index pulse to reset encoder count */ | ||
| hal_s32_t oldreading; /* used to detect overflow / underflow of the counter JE001 */ | ||
| rtapi_s64 oldreading; /* used to detect overflow / underflow of the counter JE001 AP001*/ | ||
| unsigned int indres; /* copy of reset-on-index register bits (only valid on 1st encoder of board)*/ | ||
| unsigned int indrescnt; /* counts servo cycles since index reset was turned on */ | ||
| hal_float_t *vel; /* output: scaled velocity */ | ||
|
|
@@ -1076,7 +1078,7 @@ static void read_encoders(slot_data_t *slot) | |
| int i, byteindex, byteindx2; | ||
| double vel; // local temporary velocity | ||
| union pos_tag { | ||
| hal_s32_t l; // JE001 | ||
| rtapi_s64 l; // JE001 AP001 | ||
| struct byte_tag { | ||
| signed char b0; | ||
| signed char b1; | ||
|
|
@@ -1106,16 +1108,16 @@ static void read_encoders(slot_data_t *slot) | |
| for (i = 0; i < 4; i++) { | ||
| slot->encoder[i].indrescnt++; /* increment counter each servo cycle */ | ||
| oldpos.l = slot->encoder[i].oldreading; | ||
| pos.l = *(slot->encoder[i].count); // init the higher-order bytes | ||
| pos.byte.b0 = (signed char)slot->rd_buf[byteindex++]; | ||
| pos.byte.b1 = (signed char)slot->rd_buf[byteindex++]; | ||
| pos.byte.b2 = (signed char)slot->rd_buf[byteindex++]; | ||
|
Comment on lines
1112
to
1114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are reading a value from a stream. The better method is to shift the value in byte-by-byte into the larger type instead of using a unonized type. If the value you stream-read is a two's complement value, then there is a second advantage. You can do proper sign extension in one go by casting the MSB to a signed char and then to the signed larger type. Shifting left will preserve the sign. |
||
| pos.byte.b3 = oldpos.byte.b3; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deletion of |
||
| /* check for - to + transition */ | ||
| if ((oldpos.byte.b2 & 0xc0) == 0xc0 && (pos.byte.b2 == 0)) | ||
|
Comment on lines
1115
to
1116
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What exactly is the counter's width and signedness? Masking two bits that must be set looks strange. If this two's complement, one's complement or another format?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The value read from the FPGA is an unsigned 16 bit number. So the positive wrap looks like:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But that means a simple unsigned or two's complement signed 16 bit value. Then the proper procedure would be the same as for the 32 bit case, but simply with a 16 bit sign-extended starting point: rtapi_s32 oldval = _initval_;
rtapi_s64 bigcounter = _initval_;
...
rtapi_s32 newval = (((rtapi_s32)(signed char)MSB) << 8) + (LSB & 0xff);
rtapi_s32 delta = oldval - newval;
oldval = newval;
bigcounter += delta;The first MSB cast makes the value a signed type and then the next rtapi_s32 cast forces sign-extension. And the assumption is that it does not wrap within a servo period (should be safe with just over 65 MHz at 1ms servo thread).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That could be interpreted as signed two's complement or unsigned. |
||
| pos.byte.b3++; | ||
| pos.l += 0x01000000; | ||
| else | ||
| if ((oldpos.byte.b2 == 0) && (pos.byte.b2 & 0xc0) == 0xc0) | ||
| pos.byte.b3--; | ||
| pos.l -= 0x01000000; | ||
|
Comment on lines
1115
to
+1120
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Byte-testing is a bad option. Better to look at the value as a whole and mask unwanted bits using a binary and (&). Then the compiler can sort it out properly. |
||
| *(slot->encoder[i].delta) = pos.l - slot->encoder[i].oldreading; | ||
| vel = (pos.l - slot->encoder[i].oldreading) / | ||
| (read_period * 1e-9 * slot->encoder[i].scale); | ||
|
|
@@ -1135,9 +1137,9 @@ static void read_encoders(slot_data_t *slot) | |
| /* need to properly set the 24->32 bit extension byte */ | ||
| if ( pos.byte.b2 < 0 ) { | ||
| /* going backwards */ | ||
| pos.byte.b3 = 0xFF; | ||
| pos.l |= 0xFFFFFFFFFF000000; | ||
| } else { | ||
| pos.byte.b3 = 0; | ||
| pos.l &= 0x0000000000FFFFFF; | ||
| } | ||
| oldpos.byte.b3 = pos.byte.b3; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,12 +171,14 @@ typedef struct { | |
| typedef struct { | ||
| /* counter data */ | ||
| hal_s32_t *count[MAX_CHANS]; /* captured binary count value */ | ||
| hal_s32_t offset[MAX_CHANS]; /* offset to hold latched position from index pulse */ | ||
| rtapi_s32 old_count[MAX_CHANS]; /* used for bit-extension to 32 bits */ | ||
| hal_s64_t *count64[MAX_CHANS]; /* 64-bit counts */ | ||
| rtapi_s64 offset[MAX_CHANS]; /* offset to hold latched position from index pulse */ | ||
| hal_float_t *pos[MAX_CHANS]; /* scaled position (floating point) */ | ||
| hal_float_t pos_scale[MAX_CHANS]; /* parameter: scaling factor for pos */ | ||
| hal_bit_t *index_enable[MAX_CHANS]; /* pins for index homing */ | ||
| hal_bit_t *index_latch[MAX_CHANS]; /* value of the index latch for the axis */ | ||
| // hal_s32_t check_index[MAX_CHANS]; /* internal marker for two stage index pulse check */ | ||
| hal_bit_t *index_latch[MAX_CHANS]; /* value of the index latch for the axis */ | ||
| // hal_s32_t check_index[MAX_CHANS]; /* internal marker for two stage index pulse check */ | ||
| hal_bit_t *index_polarity[MAX_CHANS]; /* Polarity of index pulse */ | ||
|
|
||
| /* dac data */ | ||
|
|
@@ -528,8 +530,9 @@ static void stg_counter_capture(void *arg, long period) | |
| // read the value without latching, latching was done on index | ||
| // remember this as an offset, it will be substracted from nominal | ||
| stg->offset[n] = stg_counter_read(n); | ||
| /* set index-enable false, so outside knows we found the index, and reset the position */ | ||
| *(stg->index_enable[n]) = 0; | ||
| stg->counts64[n] = 0 - stg->offset[n]; // atp #2331 14/11/25 | ||
| /* set index-enable false, so outside knows we found the index, and reset the position */ | ||
| *(stg->index_enable[n]) = 0; | ||
|
|
||
| /* | ||
| msg = rtapi_get_msg_level(); | ||
|
|
@@ -576,10 +579,11 @@ static void stg_counter_capture(void *arg, long period) | |
| if ( *(stg->index_enable[n]) == 1 ) | ||
| { | ||
| // read the value without latching, latching was done on index | ||
| // remember this as an offset, it will be substracted from nominal | ||
| // remember this as an offset, it will be substracted from nominal | ||
| stg->offset[n] = stg_counter_read(n); | ||
| /* set index-enable false, so outside knows we found the index, and reset the position */ | ||
| *(stg->index_enable[n]) = 0; | ||
| stg->counts64[n] = 0 - stg->offset[n]; // atp #2331 14/11/25 | ||
| /* set index-enable false, so outside knows we found the index, and reset the position */ | ||
| *(stg->index_enable[n]) = 0; | ||
|
|
||
| /* | ||
| msg = rtapi_get_msg_level(); | ||
|
|
@@ -603,10 +607,14 @@ static void stg_counter_capture(void *arg, long period) | |
|
|
||
| for (n = 0; n < num_chan; n++) | ||
| { | ||
| rtapi_s32 delta; | ||
| /* capture raw counts to latches */ | ||
| stg_counter_latch(n); | ||
| /* read raw count, and substract the offset (determined by indexed homing) */ | ||
| *(stg->count[n]) = stg_counter_read(n) - stg->offset[n]; | ||
| *(stg->count[n]) = stg_counter_read(n) - stg->offset[n]; | ||
| // 64-bit index is handled differently | ||
| delta = (stg_counter_read(n) - (stg->old_count[n]); | ||
| *(stg->count64[n]) += delta; | ||
| /* make sure scale isn't zero or tiny to avoid divide error */ | ||
| if (stg->pos_scale[n] < 0.0) { | ||
| if (stg->pos_scale[n] > -EPSILON) | ||
|
|
@@ -616,7 +624,7 @@ static void stg_counter_capture(void *arg, long period) | |
| stg->pos_scale[n] = 1.0; | ||
| } | ||
| /* scale count to make floating point position */ | ||
| *(stg->pos[n]) = *(stg->count[n]) / stg->pos_scale[n]; | ||
| *(stg->pos[n]) = *(stg->count64[n]) / stg->pos_scale[n]; | ||
|
Comment on lines
617
to
+627
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Writing a hal pin and reading the same pin just a bit later is very inefficient because the compiler must physically read the value from memory. The volatile modifier prevents caching. However, the value does not change here between the two lines. Therefore, a local intermediary allows the compiler to be more efficient. This issue is seen all over this and other code, writing and then shortly thereafter again reading same pin. |
||
| } | ||
| /* done */ | ||
| return; | ||
|
|
@@ -1435,12 +1443,17 @@ static int export_counter(int num, stg_struct *addr) | |
| msg = rtapi_get_msg_level(); | ||
| rtapi_set_msg_level( STG_MSG_LEVEL ); | ||
|
|
||
| /* export pin for counts captured by update() */ | ||
| /* export pins for counts captured by update() */ | ||
| retval = hal_pin_s32_newf(HAL_OUT, &addr->count[num], | ||
| comp_id, "stg.%d.counts", num); | ||
| if (retval != 0) { | ||
| return retval; | ||
| } | ||
| retval = hal_pin_s64_newf(HAL_OUT, &addr->count64[num], | ||
| comp_id, "stg.%d.counts64", num); | ||
| if (retval != 0) { | ||
| return retval; | ||
| } | ||
| /* export pin for scaled position captured by update() */ | ||
| retval = hal_pin_float_newf(HAL_OUT, &addr->pos[num], | ||
| comp_id, "stg.%d.position", num); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local variables must not use HAL types.