Closed GoogleCodeExporter closed 8 years ago
A bit of errata and more details:
The received characters before locking-up are "ate0\r\n" and not "ate0\n\r".
The line of code that needs changing is in the sendCmd function just before
sending a command. The code currently is: cmd += "\r\n"; but removing the '\n'
improves compatibility with some adapters so maybe it can be coded as an option
if not enabled by default.
Original comment by i.anto...@gmail.com
on 19 Apr 2011 at 9:52
I did a bit more debugging and it seems that after removing the '\n' character
from the sent command and adding a 100ms sleep just before sending the ate0
command in the while loop from the startDevice() function in the
ObdConnectThread, my adapter is actually working. Reducing the sleep to just
50ms was not enough to cure the lockups. I assume that myparticular adapter
requires some delay between bluetooth being connected and being able to
properly respond to commands.
Additionally, for some adapters, if you get an error notification just once at
the begining it might be just because the first command might read just the
init sequence "ELM v..." that can be sent unsollicited by the adapter on the
first connection after power up. To cure this, I added in my code, a small loop
to check for available characters just before sending a command.
Original comment by i.anto...@gmail.com
on 20 Apr 2011 at 7:47
Can you provide a patch for analysis?
Thank you,
PP
Original comment by pjpi...@gmail.com
on 15 Sep 2011 at 6:56
Original comment by pjpi...@gmail.com
on 22 Sep 2011 at 11:15
Hi PP,
Nice to see the project is alive :)
I have not worked with the latest version yet, but
I'll try to integrate my changes into the latest code and provide (within 1
week) at least the changed files. Regarding the patch, what parameters do I
need to run patch with (on windows) to be suitable for you?.
All the best,
Iosif
Original comment by i.anto...@gmail.com
on 22 Sep 2011 at 11:29
Hi Iosif,
Glad you're still interested in this project :-)
Anyway, I believe there's no more the need for a patch as I think I've fixed
this. I just haven't been able to commit the changes as I'm performing an heavy
refactoring to the code (see issue #10) and everything is broken atm. I'll have
it done by the end of the next week.
Feel free to follow the current dev-1_3 branch and try it yourself.
Also, just for the sake of testing, check this
[http://dl.dropbox.com/u/143834/obd-reader-1.1.2.1.apk APK] out. The values
will be shown as hexadecimal but you can see if everything is working and send
some feedback.
Much appreciated,
PP
Original comment by pjpi...@gmail.com
on 22 Sep 2011 at 12:03
Hi,
Great news, I have tried the new custom apk and it seems to work OK (I said
seems because there are still some errors if the protocol is reset to auto and
the device responds with the "SEARCHING" message. Other than that it works.
I saw you changed the eol to be just '\r' which is essential for my device and
you also added a sleep for 500ms.
The 500ms might me a bit too much for some devices that do not need it, and
maybe at some point it will become a configuration option (in my old modified
code, I had a new settings group named compatibility and there I added this
configurable delay).
One more note regarding the custom apk, If I try running a single command
(e.g., Engine RPM), it seems I get no readable answer (I get all kind of
sending line feeds off and timeout commands OK, but no real result. Maybe that
is normal at this stage.
I tried compiling the dev branch but as you also mentioned, it did not compile
properly when I tried it so I could not create a patch and it seems that you
already have the code in the right place.
All the best,
Iosif.
Original comment by i.anto...@gmail.com
on 27 Sep 2011 at 9:28
Glad to hear it worked. Now, about your comments:
Firstly, the 500ms hard-coded delay is going to be removed completely, since
I've implemented a "TIME OUT" command "AT ST hh" (hh min=0x01, max=0xFF) that
will set the maximum time (min=1, max=1200ms) the ELM device wait for the ECU
to respond to any given command.
The ELM device can even adapt "automagically", but the next release we'll be
hard-coding a predetermined value.
Secondly, this is the "expected" result for the custom version you just tested:
when running single commands, the communication with the ELM device will be
reset. The result itself may vary. Next release (version 1.3) will fix this.
Thirdly, the dev_1-3 branch is outdated until I get a working APK. There has
been an heavy refactoring (extract commands stuff to pure Java API, rewrite the
way the Android app establishes and maintains permanent ELM connection,
unit-testing, etc.). So, you understand why I'm not committing anything..
As I've said, I'm expecting a release candidate version around this weekend.
Original comment by pjpi...@gmail.com
on 28 Sep 2011 at 7:21
Hi PP,
As the 1.3 release seems to be based on quite a major refactoring of code, I
would expect it would take some time until some bits are ironed out, and
whenever it is ready please let me know and I'll give it a try. I definitely do
no ask for you to rush it out, and I do understand that refactoring can take
some time (usually it is about 2X you think it might take) :).
Regarding the complete removal of the delay, it might still generate problems
even if the timeout is implemented. In my case, without any sleep I simply do
not get a complete answer and the application locks up trying to get the line
terminator.
If this would have been a problem with the AT ST command (like a timeout too
short), then I would have expected to receive a NO_DATA first before the
lockup. Again, I reiterate, whenever a new version is ready please let me know
and I will provide feedback based on my devices.
The following part of the reply it is a bit off topic but related to the
refactoring you described. I saw you mentioned permanent ELM connections, In my
opinion this is the way to go if real time visualisation is desired, and I was
wondering why the app disconnects after every round of pid reads. I assumed it
was for low power operation (e.g., if you read a set of pids every 10 secs,
having the connection off in between the reads might save some power or even
help if you have problems with "noisy" nearby RF (if you are connected less
time to the obd device, there are less chances to lose the connection).
Obviously as it is hard to please everyone maybe it will be configurable at
some point. :)
Also regarding this issue, I remember noticing on my older version that each
obd command had its own thread when it was run (likely because it had that
start call and a thread can not be restarted once finished). This behavior is
bit far from optimal as it is creating/destroying quite a considerable number
of threads. However, I considered this so far as a minor inconvenient compared
to a lockup. :)
When you refactor the bt connection, maybe just as a suggestion, the code can
be changed to have a reading thread and a write thread that are going in loops
and another one that is submitting commands. This approach should enable
saving some overhead.
Once again thank you for the hard work on the project!
Iosif
Original comment by i.anto...@gmail.com
on 30 Sep 2011 at 4:36
Iosif, summing up I pretty much agree with everything you stated, and I can
assure you there's been a lot of code changes in the past two weeks regarding
these ideas.
Now, some comments on your thoughts..
Speaking about the major refactoring, so far the API is complete with some
unit-testing going on. It is a combination of pure Java classes where I
implement the OBD commands logic (sending command to OutputStream, reading
response from InputStream and interpreting the result).
At the moment I'm refactoring the Android part (Activities, Services,
BluetoothSockets, threads, etc.) and I won't commit to the public repo before
the Android app is 100% functional with the new API.
Next, the delay functionality. Well, what you probably don't know is that the
ELM interface will give a "NO DATA>" response if your car ECU doesn't respond.
So, I believe the "AT ST hh" command will suffice! It wasn't implemented in
previous versions of this project. Let's wait and see when 1.3 comes out. I'm
counting on your feedback, that's for sure!
Also, I'm going for permanent connection and sequential command execution (send
and receive), thus not having multi-threaded/asynchronous logic going on. The
ELM works this way, so trying to do it differently is just a waste of resources
and added complexity.
And yes, yes, yes!! You're very right on what concerns the threading model
implemented so far. Once again, my refactoring is removing that part.
Given all these changes, I can only ask your for some patience as something is
coming out. I just want to be properly tested before I commit something to the
public tree.
Thank you so much for your feedback and keeping up with me,
Paulo Pires
Original comment by pjpi...@gmail.com
on 30 Sep 2011 at 7:34
This issue was closed by revision 5fec2de19ee1.
Original comment by pjpi...@gmail.com
on 28 Oct 2011 at 9:13
1.3-RC1 is available in the Downloads section. Test it and let me know.
Original comment by pjpi...@gmail.com
on 29 Oct 2011 at 10:46
Hi,
I have tested 1.3-RC1 and it seems to work. And by work I mean I could get some
readings in the main screen but all the readings were for the same parameter
and maybe that is expected. Thus, I confirm I did not had the lockup on start
as it used to be the case previously.
All the best,
Iosif
Original comment by i.anto...@gmail.com
on 6 Nov 2011 at 3:29
Original issue reported on code.google.com by
i.anto...@gmail.com
on 19 Apr 2011 at 9:42