Skip to content

Commit

Permalink
Fix negative encoder range
Browse files Browse the repository at this point in the history
So if you install your encoder on the "other" side, you need
to configure it as a negative resolution.  The old code handled
this very badly and effectively returned really bad results if
you configured a negative resolution.

We also write a bunch of unit tests to validate the values are now
correct.
  • Loading branch information
synfinatic committed Jun 30, 2017
1 parent 977b4be commit 368d3d9
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 18 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -6,3 +6,4 @@
*.job
*.pro
eagle.epf
*.sw?
34 changes: 19 additions & 15 deletions src/teensy_dsc/dsc.ino
Expand Up @@ -27,40 +27,44 @@
* So depending on the resolution, it either returns a
* uint16_t value or int16_t value.
*
* So if you have a resolution < 2^15, then the range is:
* -resolution to (resolution-1)
* So if you have an abs(resolution) < 2^15, then the range is:
* -resolution/2 to (resolution/2-1)
*
* If your resolution is larger, then the range is:
* 0 to 2^16-1
*
* It's so far undocumented how encoders mounted backwards
* with a negative resolution should be handled for the
* latter situation. I decided to do the semi-obvious thing
* of wrapping between 0-(Resolution - 1)
* of counting backwards.
*/

int32_t
ngc_convert_encoder_value(int32_t encoder, long resolution) {
int32_t ret, half_res;
half_res = resolution / 2;

// Different math if resolution can be stored in int16_t
if ((resolution < INT16_MAX) && (resolution >= INT16_MIN)) {
half_res = resolution / 2;
ret = encoder % resolution;
if (ret > (half_res-1)) {
if (ret > (half_res - 1)) {
ret = ret - (half_res - 1) - half_res;
} else if (ret < -half_res) {
ret = ret + (half_res * 2);
ret = ret + (half_res + 1);
}
} else {
// use UNIT_MAX resolution
ret = encoder % abs(resolution);
// if encoder value is out of range of the resolution,
// then wrap it
if ((ret < 0) && (resolution > 0)) {
ret = resolution + ret;
} else if ((ret > 0) && (resolution < 0)) {
ret = resolution - ret;
if (resolution > 0) {
ret = encoder % abs(resolution);
// use UNIT_MAX resolution
// if encoder value is out of range of the resolution,
// then wrap it
if (ret < 0) {
ret = resolution + ret;
}
} else {
ret = -encoder % abs(resolution);
if (ret < 0) {
ret = (resolution * -1) + ret;
}
}
}
return ret;
Expand Down
1 change: 1 addition & 0 deletions src/teensy_dsc/teensy_dsc.ino
Expand Up @@ -42,6 +42,7 @@ Encoder EncoderDEC(CHAN_A_DEC, CHAN_B_DEC);
AnySerial UserSerial(&USER_SERIAL_PORT);
AnySerial WiFlySerialPort(&WIFLY_SERIAL_PORT);
WiFly _WiFly(WiFlySerialPort);
cmd_status process_cmd(cli_ctx *ctx);

/* our global contexts */
cli_ctx *uctx, *wctx;
Expand Down
62 changes: 59 additions & 3 deletions src/unit_tests/unit_tests.ino
Expand Up @@ -28,13 +28,27 @@
assertEqual(ret, CHECK);\
}

#define TEST_N10K(INPUT, CHECK) {\
int ret;\
ret = ngc_convert_encoder_value(INPUT, -10000);\
assertEqual(ret, CHECK);\
}

#define TEST_40K(INPUT, CHECK) {\
int ret;\
ret = ngc_convert_encoder_value(INPUT, 40000);\
assertEqual(ret, CHECK);\
}

#define TEST_N40K(INPUT, CHECK) {\
int ret;\
ret = ngc_convert_encoder_value(INPUT, -40000);\
assertEqual(ret, CHECK);\
}

/*
* Resolution +10,000
*/
test(test_10k_0) TEST_10K(0, 0)
test(test_10k_1) TEST_10K(1, 1)
test(test_10k_1000) TEST_10K(1000, 1000)
Expand All @@ -52,6 +66,29 @@ test(test_10k_n24999) TEST_10K(-24999, -4999)
test(test_10k_n25000) TEST_10K(-25000, -5000)
test(test_10k_n25001) TEST_10K(-25001, 4999)

/*
* Resolution -10,000
*/
test(test_n10k_0) TEST_N10K(0, 0)
test(test_n10k_1) TEST_N10K(1, 9999)
test(test_n10k_1000) TEST_N10K(1000, 9000)
test(test_n10k_3000) TEST_N10K(3000, 7000)
test(test_n10k_4999) TEST_N10K(4999, 4999)
test(test_n10k_5000) TEST_N10K(5000, 4999)
test(test_n10k_25000) TEST_N10K(25000, -4999)
test(test_n10k_n1) TEST_N10K(-1, -1)
test(test_n10k_n1000) TEST_N10K(-1000, -1000)
test(test_n10k_n3000) TEST_N10K(-3000, -3000)
test(test_n10k_n4999) TEST_N10K(-4999, -4999)
test(test_n10k_n5000) TEST_N10K(-5000, -5000)
test(test_n10k_n5001) TEST_N10K(-5001, 4999)
test(test_n10k_n24999) TEST_N10K(-24999, -4999)
test(test_n10k_n25000) TEST_N10K(-25000, -5000)
test(test_n10k_n25001) TEST_N10K(-25001, 4999)

/*
* Resolution +40,000
*/
test(test_40k_0) TEST_40K(0, 0)
test(test_40k_1) TEST_40K(1, 1)
test(test_40k_1000) TEST_40K(1000, 1000)
Expand All @@ -62,16 +99,35 @@ test(test_40k_35000) TEST_40K(35000, 35000)
test(test_40k_39999) TEST_40K(39999, 39999)
test(test_40k_40000) TEST_40K(40000, 0)
test(test_40k_75000) TEST_40K(75000, 35000)
test(test_40k_n75000) TEST_40K(-75000, 5000)
test(test_40k_1200000) TEST_40K(1200000, 0)

test(test_40k_n1) TEST_40K(-1, 39999)
test(test_40k_n1) TEST_40K(-1, 39999)
test(test_40k_n1000) TEST_40K(-1000, 39000)
test(test_40k_n3000) TEST_40K(-3000, 37000)
test(test_40k_n4999) TEST_40K(-4999, 35001)
test(test_40k_n15000) TEST_40K(-15000, 25000)
test(test_40k_n35001) TEST_40K(-35001, 4999)
test(test_40k_n39999) TEST_40K(-39999, 1)
test(test_40k_n40000) TEST_40K(-40000, 0)
test(test_40k_n40001) TEST_40K(-40001, 39999)
test(test_40k_n75000) TEST_40K(-75000, 5000)
test(test_40k_n1200000) TEST_40K(-1200000, 0)

/*
* Resolution -40,000
*/
test(test_n40k_0) TEST_N40K(0, 0)
test(test_n40k_1) TEST_N40K(1, 39999)
test(test_n40k_2) TEST_N40K(2, 39998)
test(test_n40k_5000) TEST_N40K(5000, 35000)
test(test_n40k_19999) TEST_N40K(19999, 20001)
test(test_n40k_20000) TEST_N40K(20000, 20000)
test(test_n40k_20001) TEST_N40K(20001, 19999)
test(test_n40k_39999) TEST_N40K(39999, 1)
test(test_n40k_40000) TEST_N40K(40000, 0)
test(test_n40k_41001) TEST_N40K(40001, 39999)
test(test_n40k_50001) TEST_N40K(50001, 29999)
test(test_n40k_60000) TEST_N40K(60000, 20000)
test(test_n40k_60001) TEST_N40K(60001, 19999)

test(int32_long) {
assertEqual(sizeof(int32_t), sizeof(long));
Expand Down

0 comments on commit 368d3d9

Please sign in to comment.