FreeEMS Issues - Tunix
View Issue Details
0000276TunixGeneral Featurespublic2011-09-25 23:302012-03-30 20:38
unknown, updating issue to appear in roadmap
0000276: Make JimStim Minimum RPM Values Consistent
Currently you can enter 0 - 65535 in the numeric area, where 0 - 59 mean OFF and 60 up mean that many RPM. This is probably the best solution as it's nice to be able to tell it to be off without doing anything special.

However, the new slider limits you from 100 - 65535 and it would be very nice to be able to tweak that back to 60, or even lower and get OFF from it.

And the sweep lower RPM value is capped at 61 - 65535, which is wrong, because 60 is a valid value. Additionally I'd like to be able to include a dead period in the sweep, allowing 0 - 59 would achieve that nicely and make testing sync and desync possible with the interface.

Finally, the slider, while a fantastic idea, which I love, is very twitchy as is. I would suggest limiting the slider range from 0 - 20,000 such that it can do off and high rpm bike ranges. Someone who wants a higher RPM (which the JimStim probably can't support anyway) can always enter it in the raw field or the sweeper anyway.

So, my vote, field, slider, sweeper limits all at 0 for lower and slider artificially limited to 20k (or lower, but not less than 10k) to make it more usable.
No tags attached.

2011-09-26 00:22   
Actually its correct as it is. Its invaliid to have a start and end point being the same (60 60), hence the end point is always ONE higher (makes the math easier and prevents divide by zero shit).

The slider IS SUPPOSED TO BE GLITCHY, the comms speed is highly limited to jimstim, and cannot cope with a realtime updating slider which generates events about5-10x faster than can be accepted by jimstim (limited by serial mostly) which causes numerous problems (gui stall/sluggish due to the event pileup). So it's "delayed" update slider that doesn't sent an event until you either let go it of or stop moving for several hundred milliseconds. Another note: the slider MUST have the same range as the entry as they are linked together to the same ECU memory location. If I reduce the slider's range, the text field will be locked down as well thus eliminating the ability to get to 65535 which re-enabled the pots on the stim.
2011-09-26 00:45   
do you even read the comments???
2011-09-26 01:03   
Yes, and then I fix it myself. I'll close this if you pull the commit in.
2011-09-26 01:18   
Commit REJECTED, as this breaks the ability to get to the "pot mode" (enables the pots on the Stim) which requires a value of 65535 to be entered..

Go rework it to not LOOSE functionality
2011-09-26 01:25   
I knew that I was removing the ability to do that. Which didn't bother me at all, with the slider on the screen and the ability to reach over, flick the dip switches, whip off the serial cable, and pull off the jumpers to make them work if/when required. A check box for "re-enable pots" would be much more clear anyway. If you know how to let the text box be 65535 and leave the slider at 0 - 10k, great. I do not and don't wish to spend time on it when it does what I need. Jimstim glade/datamap files will be low churn anyway.
2011-09-26 02:19   
Not acceptable. U broke functionality that worked previously.
2011-09-26 07:45   
Acceptable to me, this is open source remember, please take what I did, and make it do what you want too, and then everyone can be happy little bunnies.
2011-09-26 10:58   
No your commit is a step backwards. You fix it keeping in mind there are OTHER PEOPLE that use it....
2011-09-26 11:07   
If my work is not useful to you, that is OK. It is useful to me. I will keep it as-is. You don't have to take my commit, it can sit there also as-is harming no one. If you want to make the app do what I need it to do (is anyone else really using this on a daily basis, all day, every day?) that would be great, otherwise, for me, it already does.
2011-09-26 11:35   
This isn't about just you. OTHERS (me included) use jimstim mode, and you BROKE IT, If I recall from the jimstim guy, setting a RPM value below 60 was unwise as it causes overflow problems for various high tooth wheels, which results in confused tuner and ECU.
2011-09-26 11:53   
I didn't break it, I removed a useless feature and improved usability of other features a LOT. As for setting rpm below 60, that doesn't bother me if there is a check box that says off and another that says "pause with output off for X time at low end of sweep". Otherwise, I'd like to see your reference on that comment, and even if you can find one, it works as intended for all modes that I've tried, AND, you were/are allowing it in the text field with manual entry. You can clearly do a better job of modifying the app that you designed than I can, so go ahead and do it and stop spending both of our time here arguing the point. I'm satisfied with what I have, and won't be working on it further, as I have no need for such work. If you roll forward and break what I have, I'll keep this version for my testing as-is.
2011-09-26 12:12   
It does NOT PAUSE there (below 60 rpm), it generates random fucked up RPM values due to counters overflowing in the jimstim. So when it drops below 60 depending on the pattern chosen, the rpm value synthesized is completely ambiguous, it might be zero, or any number between 0 and 65534 RPM.

That is what I call a "shit user experience", and will not put that in.
2011-09-26 12:15   
I didn't ask you to, re-read what I wrote. I asked you to ADD a pause there, in lieu of having an effective pause (for at least some patterns) there. I'm yet to see such random RPM values, nor have I seen your reference for this behaviour from one of the two authors. Given that the firmware is closed source, I'd need to see one of those things to believe it.
2011-09-27 10:48   
Awesome work, Dave! Love it! I tweaked the lower and upper bounds of the sweep RPM figures and increased the step size max to 2k and I'm totally happy with it. If you take this commit, I'll close this.
2011-09-27 13:18   
Everything is very usable now! Excellent work.