collin80 / GEVCU

Generalized Electric Vehicle Control Unit
113 stars 56 forks source link

Throttle difficult to use with single pot pedal #20

Closed collin80 closed 11 years ago

collin80 commented 11 years ago

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

jrickard commented 11 years ago

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder notifications@github.comwrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

— Reply to this email directly or view it on GitHubhttps://github.com/collin80/GEVCU/issues/20 .

http://www.EVTV.me http://EVTV.me

Electric Vehicle Television - KickinGas - One Car at a Time.

jrickard commented 11 years ago

and polarity

On Fri, Aug 2, 2013 at 10:53 AM, Jack Rickard mjrickard@gmail.com wrote:

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder notifications@github.comwrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

— Reply to this email directly or view it on GitHubhttps://github.com/collin80/GEVCU/issues/20 .

http://www.EVTV.me http://EVTV.me

Electric Vehicle Television - KickinGas - One Car at a Time.

http://www.EVTV.me http://EVTV.me

Electric Vehicle Television - KickinGas - One Car at a Time.

cgalpin commented 11 years ago

The calibration does all this. I think what you are seeing is we don't save this in the preferences we need to save the number of pots and the style if 2.

-- charles

On Aug 2, 2013, at 11:56 AM, Jack Rickard notifications@github.com wrote:

and polarity

On Fri, Aug 2, 2013 at 10:53 AM, Jack Rickard mjrickard@gmail.com wrote:

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder notifications@github.comwrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

— Reply to this email directly or view it on GitHubhttps://github.com/collin80/GEVCU/issues/20 .

http://www.EVTV.me http://EVTV.me

Electric Vehicle Television - KickinGas - One Car at a Time.

http://www.EVTV.me http://EVTV.me

Electric Vehicle Television - KickinGas - One Car at a Time. — Reply to this email directly or view it on GitHub.

cgalpin commented 11 years ago

So where do you stand with the throttle Jack?

I'll take some of the blame for this stuff not working as you'd expect since I introduced the calibration, but I feel the real issue is that the calibration really hasn't been integrated or used fully - we never came to an agreement on storing the data collected. I am not sure if I have made my point yet about the preferences needing to store more info but judging from stuff I have read lately I think so (sorry lots of messages over the last few days and I scanned some from my phone).

The detection establishes a bunch of stuff and we need to store and honor that for it to be useful. The calibration should have detected only one throttle for you, and if we had saved it and used it then it should have been transparent, although I'd advise people to ground the second pin if not using it. Did the calibration show one pot for you?

If I add the ability to detect if the second pot is double the first or inverse, we should store this and then use it. Etc. Someone should be able to plug in a throttle, run the calibration and either have confidence it's a know pedal style and it just work, or be given a clear message that their pedal is unsupported and they should email for help getting it supported or RTFM for more info.

I'd like to focus on getting Jack going right now as that gives us 3 pot throttles and on CAN throttle to make sure we have this architect right and then agree on a solution and fix it once and for all. I feel like we have been dodging the real issue and we should either add the preferences or come up with another solution so I can shut up about it :)

Still looking forward to seeing you get the THING going before Thursday!

charles

On Aug 2, 2013, at 11:56 AM, Jack Rickard wrote:

and polarity

On Fri, Aug 2, 2013 at 10:53 AM, Jack Rickard mjrickard@gmail.com wrote:

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder notifications@github.comwrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

collin80 commented 11 years ago

I'm leaning toward storing more things in preference but I still really hope that storing the value at top of pedal for 2 pots and the value at fully pressed pedal for two pots is enough. If there is only one actual pot then set the second one to all zeros. This actually already happens in the most recent code. But, right now the throttle code doesn't care. It will use how ever many pots you told it to when instantiating the object. That's naughty. Once it reads in the throttle min/max values it should automatically use the proper # of throttles. Actually, we shouldn't be passing ADC numbers to the throttle code at all. Those should be stored in EEPROM and automatically loaded on start up. There would also be a good place to set the # of throttles. If one of the ADC values is 255 then don't use it. That way everything is configurable.

This all, however, glosses over how you're pretty sure your pedal doesn't neatly fit the mold. I believe you sent some logging of your pedal operating but I've since lost it in the avalanche of messages. If it really is totally different then perhaps we create a new subclass of Throttle that works differently. If we do that then PotThrottle is probably improperly named. Perhaps it should change to LinearPotThrottle or something to indicate how it assumes that the 1 or 2 ADC values it gets will linearly scale either up or down across the range. A pedal which does not do this will cause problems.

On Mon, Aug 5, 2013 at 1:15 PM, cgalpin notifications@github.com wrote:

So where do you stand with the throttle Jack?

I'll take some of the blame for this stuff not working as you'd expect since I introduced the calibration, but I feel the real issue is that the calibration really hasn't been integrated or used fully - we never came to an agreement on storing the data collected. I am not sure if I have made my point yet about the preferences needing to store more info but judging from stuff I have read lately I think so (sorry lots of messages over the last few days and I scanned some from my phone).

The detection establishes a bunch of stuff and we need to store and honor that for it to be useful. The calibration should have detected only one throttle for you, and if we had saved it and used it then it should have been transparent, although I'd advise people to ground the second pin if not using it. Did the calibration show one pot for you?

If I add the ability to detect if the second pot is double the first or inverse, we should store this and then use it. Etc. Someone should be able to plug in a throttle, run the calibration and either have confidence it's a know pedal style and it just work, or be given a clear message that their pedal is unsupported and they should email for help getting it supported or RTFM for more info.

I'd like to focus on getting Jack going right now as that gives us 3 pot throttles and on CAN throttle to make sure we have this architect right and then agree on a solution and fix it once and for all. I feel like we have been dodging the real issue and we should either add the preferences or come up with another solution so I can shut up about it :)

Still looking forward to seeing you get the THING going before Thursday!

charles

On Aug 2, 2013, at 11:56 AM, Jack Rickard wrote:

and polarity

On Fri, Aug 2, 2013 at 10:53 AM, Jack Rickard mjrickard@gmail.com wrote:

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder < notifications@github.com>wrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

— Reply to this email directly or view it on GitHubhttps://github.com/collin80/GEVCU/issues/20#issuecomment-22121146 .

cgalpin commented 11 years ago

Let's get Jack's working and compare all three of our logs. Mine truly is different. I am not sure a separate class is needed but suspect not. There just needs to be a couple of different ways of verifying the pot values look sane and then a 0-10000 number should be mapped/derived and the rest is identical. Keep in mind that if this is done right they shouldn't need to edit code to choose different throttle classes. I agree if it's completely different like the CAN throttle this should be separate but I think we can do a single PotThrottle

charles

On Aug 5, 2013, at 1:23 PM, Collin Kidder wrote:

I'm leaning toward storing more things in preference but I still really hope that storing the value at top of pedal for 2 pots and the value at fully pressed pedal for two pots is enough. If there is only one actual pot then set the second one to all zeros. This actually already happens in the most recent code. But, right now the throttle code doesn't care. It will use how ever many pots you told it to when instantiating the object. That's naughty. Once it reads in the throttle min/max values it should automatically use the proper # of throttles. Actually, we shouldn't be passing ADC numbers to the throttle code at all. Those should be stored in EEPROM and automatically loaded on start up. There would also be a good place to set the # of throttles. If one of the ADC values is 255 then don't use it. That way everything is configurable.

This all, however, glosses over how you're pretty sure your pedal doesn't neatly fit the mold. I believe you sent some logging of your pedal operating but I've since lost it in the avalanche of messages. If it really is totally different then perhaps we create a new subclass of Throttle that works differently. If we do that then PotThrottle is probably improperly named. Perhaps it should change to LinearPotThrottle or something to indicate how it assumes that the 1 or 2 ADC values it gets will linearly scale either up or down across the range. A pedal which does not do this will cause problems.

On Mon, Aug 5, 2013 at 1:15 PM, cgalpin notifications@github.com wrote:

So where do you stand with the throttle Jack?

I'll take some of the blame for this stuff not working as you'd expect since I introduced the calibration, but I feel the real issue is that the calibration really hasn't been integrated or used fully - we never came to an agreement on storing the data collected. I am not sure if I have made my point yet about the preferences needing to store more info but judging from stuff I have read lately I think so (sorry lots of messages over the last few days and I scanned some from my phone).

The detection establishes a bunch of stuff and we need to store and honor that for it to be useful. The calibration should have detected only one throttle for you, and if we had saved it and used it then it should have been transparent, although I'd advise people to ground the second pin if not using it. Did the calibration show one pot for you?

If I add the ability to detect if the second pot is double the first or inverse, we should store this and then use it. Etc. Someone should be able to plug in a throttle, run the calibration and either have confidence it's a know pedal style and it just work, or be given a clear message that their pedal is unsupported and they should email for help getting it supported or RTFM for more info.

I'd like to focus on getting Jack going right now as that gives us 3 pot throttles and on CAN throttle to make sure we have this architect right and then agree on a solution and fix it once and for all. I feel like we have been dodging the real issue and we should either add the preferences or come up with another solution so I can shut up about it :)

Still looking forward to seeing you get the THING going before Thursday!

charles

On Aug 2, 2013, at 11:56 AM, Jack Rickard wrote:

and polarity

On Fri, Aug 2, 2013 at 10:53 AM, Jack Rickard mjrickard@gmail.com wrote:

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder < notifications@github.com>wrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

— Reply to this email directly or view it on GitHubhttps://github.com/collin80/GEVCU/issues/20#issuecomment-22121146 .

— Reply to this email directly or view it on GitHub.

jrickard commented 11 years ago

Charles:

I like your calibration. It isn't you at all. It's probably the hardware. I've never even used it on the test bench, just took Ed's word it would work. We're not detecting voltages on the throttle inputs. I DO get the digital inputs D0 and D1 on the enable and disable. But I don't think either RAW or nonRAW are working correctly.

I've moved it to the test bench and am wiring up a harness now. Mark Wiesheimer will be in this morning to get my ass straightened out. But I kind of know what we are going to find. It's probably a hardware issue but could be software based on switches like the commenting of RAW and the commenting of SEND SERIAL and things like that.

I would find it preposterous that it had anything to do with your code. I'm watching inputs from the L on the menu and running your calibration function and it looks great - it's just not getting a voltage reading off the input. WHat you don't have is that we do some stuff in hardware with linear optos before it ever gets to the A/D input pins. You don't have the hardware.

Mark is actually bringing me new. I'll have the test bench wired up. We can actually put meters to the board pins will moving a pot. Watch motor turn. Swap to other hardware. I should have done all this earlier. Thought I might get "lucky" instead. While it's better to be lucky than good, it's a little fickle.

We'll stop, slow down, and work it. In any event, once Collin and Ed are here, they will be able to do things on the bench, move them to the car, do things to the car, move them to the bench. It's all good. I just have to get it set up this morning.

Jack

On Mon, Aug 5, 2013 at 12:23 PM, Collin Kidder notifications@github.comwrote:

I'm leaning toward storing more things in preference but I still really hope that storing the value at top of pedal for 2 pots and the value at fully pressed pedal for two pots is enough. If there is only one actual pot then set the second one to all zeros. This actually already happens in the most recent code. But, right now the throttle code doesn't care. It will use how ever many pots you told it to when instantiating the object. That's naughty. Once it reads in the throttle min/max values it should automatically use the proper # of throttles. Actually, we shouldn't be passing ADC numbers to the throttle code at all. Those should be stored in EEPROM and automatically loaded on start up. There would also be a good place to set the # of throttles. If one of the ADC values is 255 then don't use it. That way everything is configurable.

This all, however, glosses over how you're pretty sure your pedal doesn't neatly fit the mold. I believe you sent some logging of your pedal operating but I've since lost it in the avalanche of messages. If it really is totally different then perhaps we create a new subclass of Throttle that works differently. If we do that then PotThrottle is probably improperly named. Perhaps it should change to LinearPotThrottle or something to indicate how it assumes that the 1 or 2 ADC values it gets will linearly scale either up or down across the range. A pedal which does not do this will cause problems.

On Mon, Aug 5, 2013 at 1:15 PM, cgalpin notifications@github.com wrote:

So where do you stand with the throttle Jack?

I'll take some of the blame for this stuff not working as you'd expect since I introduced the calibration, but I feel the real issue is that the calibration really hasn't been integrated or used fully - we never came to an agreement on storing the data collected. I am not sure if I have made my point yet about the preferences needing to store more info but judging from stuff I have read lately I think so (sorry lots of messages over the last few days and I scanned some from my phone).

The detection establishes a bunch of stuff and we need to store and honor that for it to be useful. The calibration should have detected only one throttle for you, and if we had saved it and used it then it should have been transparent, although I'd advise people to ground the second pin if not using it. Did the calibration show one pot for you?

If I add the ability to detect if the second pot is double the first or inverse, we should store this and then use it. Etc. Someone should be able to plug in a throttle, run the calibration and either have confidence it's a know pedal style and it just work, or be given a clear message that their pedal is unsupported and they should email for help getting it supported or RTFM for more info.

I'd like to focus on getting Jack going right now as that gives us 3 pot throttles and on CAN throttle to make sure we have this architect right and then agree on a solution and fix it once and for all. I feel like we have been dodging the real issue and we should either add the preferences or come up with another solution so I can shut up about it :)

Still looking forward to seeing you get the THING going before Thursday!

charles

On Aug 2, 2013, at 11:56 AM, Jack Rickard wrote:

and polarity

On Fri, Aug 2, 2013 at 10:53 AM, Jack Rickard mjrickard@gmail.com wrote:

Well I agree. In fact I would have it where you could put a one wire pot on either input, and if either just doesn't change at all, it is ignored and it uses the one that did change. And if both inputs changed it would use both and determine the sanity check ratio.

Jack Rickard

On Fri, Aug 2, 2013 at 10:50 AM, Collin Kidder < notifications@github.com>wrote:

If the pot pedal object is instantiated with two ADCs but you only have one throttle pot then bad things will happen as it will still try to use the second one. The code needs to look at calibration data and ignore the second ADC if the calibration reads 0.

— Reply to this email directly or view it on GitHub< https://github.com/collin80/GEVCU/issues/20#issuecomment-22121146> .

— Reply to this email directly or view it on GitHubhttps://github.com/collin80/GEVCU/issues/20#issuecomment-22121703 .

http://www.EVTV.me http://EVTV.me

Electric Vehicle Television - KickinGas - One Car at a Time.