speeduino
speeduino copied to clipboard
Auxilliary input pin initialisation bug
There is a bug with the way Aux. input pins are initialised (and to be subsequently used for programmable I/O).
If an aux. input is assigned to pin x, the pinMode(pin, type) initialisation is done on pin x-1. This is not obvious because of the way the pin indices are defined in the INI file - DIGITAL_PIN is defined as strings starting from "1" as index 0, "2" as index 1, etc. hence why the offset is needed. But other defs use 0 as "Undefined" or "Disabled".
In the speeduino.ino file, we have
currentStatus.canin[currentStatus.current_caninchannel] = readAuxdigital((configPage9.Auxinpinb[currentStatus.current_caninchannel]&63)+1);
but this pin is initialised in sensors.ino,
byte pinNumber = (configPage9.Auxinpinb[currentStatus.current_caninchannel]&127); if( pinIsUsed(pinNumber) ) { BIT_SET(currentStatus.engineProtectStatus, PROTECT_IO_ERROR); } else { pinMode( pinNumber, INPUT); auxIsEnabled = true; }
causing a problem if a). pin 36 is already used (because we actually want pin 37, but the pinIsUsed check is done on pin 36), and b). this is the only aux. channel used, meaning that auxIsEnabled will never be set to true.
You may not even notice the problem if there were other aux inputs where x-1 didn't collide, and auxIsEnabled got set to true by "accident". Additionally the pinMode defaults to INPUT.
I believe byte pinNumber = (configPage9.Auxinpinb[currentStatus.current_caninchannel]&127); should be byte pinNumber = (configPage9.Auxinpinb[currentStatus.current_caninchannel]&63) + 1;
Wow great catch! I was experiencing this problem first hand when I was setting up aux in 1 for an analog Input and i could never Get it to read. I ended up fiddling with it until it finally started showing values
https://github.com/noisymime/speeduino/pull/586#issuecomment-932878370
Is there more involved in this than changing the two instance of byte pinNumber = (configPage9.Auxinpinb[currentStatus.current_caninchannel]&127); to byte pinNumber = (configPage9.Auxinpinb[currentStatus.current_caninchannel]&63) + 1; in sensors.ino?
I applied the fix above and PR #675 and still have issue #673
I was wondering if you had anymore insight on my issue.
See my last comment on https://github.com/noisymime/speeduino/issues/673:
#654 was closed and "fixed" but whoever merged it only applied half my fix - Aux in1-16 should be Aux in 0-15 in the fullStatus defs at the top.
See my last comment on #673:
#654 was closed and "fixed" but whoever merged it only applied half my fix - Aux in1-16 should be Aux in 0-15 in the fullStatus defs at the top.
See my update on #673 as well I think there is more to it than that
Is this issue still present with current master release?
Yep, just had a look through the master branch (I haven't tested the code, just read through it, full disclosure).
Somehow all the fixes I have suggested have been undone, that is:
#define DIGITAL_PIN = "1", "2" ...
Means that
pinMode(DIGITAL_PIN "1") is actually doing pinMode(0) which is incorrect
and then in speeduino.ino
pseudo-code: readAuxdigital(DIGITAL_PIN "1" + 0) is actually readAuxdigital(0 + 1) which is correct.
(here's the full line: currentStatus.canin[currentStatus.current_caninchannel] = readAuxdigital((configPage9.Auxinpinb[currentStatus.current_caninchannel]&63)+1);)
Aaaaaaaan additionally:
#define fullStatus_def_2= "RPM DOT", ... "baro", "Aux in 1", "INVALID", "Aux in 2", "INVALID", ...
is STILL incorrect, it needs to start from "Aux in 0", "INVALID", "Aux in 1", ... "Aux in 15".
Cheers :)
Last bit is on #676