lkml.org 
[lkml]   [2009]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] staging: s5k3e2fx.c: reduce complexity by factoring
On Tue, Dec 15, 2009 at 10:37 AM, Justin Madru <jdm64@gawab.com> wrote:
> But, wouldn't you agree that if the code was suppose to deal with "rounding
> issues" that there's a
> simpler expression?

No, I don't agree. Five minutes of effort below shows your patch will
generate different numbers than the original. If this is controlling a
stepper motor trying to hit a home position, it's off now. Or, the
errors in the expressions for moving near and far may have balanced
each other out before, and now there may be a systematic error causing
a skew over time toward one end rather than the other.

My point is that you need to run this past the guy with the actual
hardware who wrote it in the first place such that it can be tested,
and make sure the slapped-together expression isn't just working by
accident, as ugly as it might be.

#include <stdio.h>

typedef int int32_t;
typedef short int16_t;
typedef unsigned int uint32_t;

enum {MOVE_NEAR, MOVE_FAR} move_direction;

int32_t s5k3e2fx_move_focus(int direction, int32_t num_steps)
{
int32_t i;
int16_t step_direction;
int16_t actual_step;
int16_t s_move[5], s_move_2[5];
uint32_t gain, gain_2;

if (direction == MOVE_NEAR)
step_direction = 20;
else
step_direction = -20;

actual_step = step_direction * (int16_t)num_steps;

gain = actual_step * 0x400 / 5;
gain_2 = actual_step / 5;

for (i = 0; i <= 4; i++) {
if (actual_step >= 0)
s_move[i] = ((((i+1)*gain+0x200) -
(i*gain+0x200))/0x400);
else
s_move[i] = ((((i+1)*gain-0x200) -
(i*gain-0x200))/0x400);
}

for (i = 0; i <= 4; i++)
s_move_2[i] = gain_2;

if (memcmp(s_move, s_move_2, sizeof(s_move))) {
printf("s1, s2 differ for direction %d, num_steps %d\n", direction,
num_steps);
for (i=0; i<5; i++)
printf(" [%d] %d %d", i, s_move[i], s_move_2[i]);
printf("\n");
}

}

int main(void) {
int steps;
for (steps = -65535; steps < 65536; steps++) {
s5k3e2fx_move_focus(MOVE_NEAR, steps);
s5k3e2fx_move_focus(MOVE_FAR, steps);
}
}



>
> Justin
>


\
 
 \ /
  Last update: 2009-12-15 20:13    [W:0.060 / U:0.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site