Fix Ignition State updating on JM-VL01 and similar devices #4816
Conversation
Updates Ignition State from 0xA0 location packet (MSG_GPS_2). This information was previously ignored, and Ignition State was only obtained from 0x13 Heartbeat Packet (MSG_STATUS). Ignition State is now updated immediately, rather than waiting considerable time for the next Heartbeat, which caused Notifications to be delayed or missed completely. Note: There are still a few readable byes in the 0xA0 packet that are ignored.
|
Please attach protocol documentation. |
|
JM-VL01 GPS Tracker Communication Protocol_v15_20200701.pdf 0xA0 documentation referenced at 5.3 (Page 17) onward. |
| @@ -964,6 +964,10 @@ private void decodeBasicUniversal(ByteBuf buf, DeviceSession deviceSession, int | |||
| if (buf.readableBytes() == 4 + 6) { | |||
| position.set(Position.KEY_ODOMETER, buf.readUnsignedInt()); | |||
| } | |||
|
|
|||
| if (type == MSG_GPS_2 && buf.readableBytes() >= 5) { | |||
There was a problem hiding this comment.
Where did number 5 come from? Looks like it should be 7, but I would actually make it more specific and just specified exactly 9. Does that make sense?
There was a problem hiding this comment.
Actually, that was me. I worked on this with @Meesen0743
It wasn't clear, and I didn't have time to debug, if readable bytes included the error bytes, or stop bytes. Without the error bytes, its 5. with, its 7. if it also includes the stop bit, its 9.
However, there is some revisions of code (v19) that also include mileage, which i believe takes up an additional 4 bytes (ill have to find the other documentation). The position of ACC is the same for both.
So depending on the firmware revision, device, and if stop/error bytes are included in readable bytes, it could be 5, 7, 9, 11, or 13.
All of those are >=5, which is where 5 came from. I agree that its not as specific as it could be.
I assume you are saying stop and error is included (which i suspected), so you could then say >=9 (because its going to be either 9 or 13)
or if you wanted to be super accurate, you could say:
type == MSG_GPS_2 && (buf.readableBytes() ==9 || buf.readableBytes() ==13)
(again, ill have to check the other documentation to ensure mileage is 4 bytes)
if you can confirm my thought process, that stop and error bytes are part of the available readable bytes, @Meesen0743 can update the PR, and upload the other revision of documentation to show why the value may differ.
There was a problem hiding this comment.
There's always at least serial number + error check + stop bit, so it's at least 6. With the field itself, it should be minimum 7. I'm OK with using 7, but 5 doesn't look right, unless I'm missing something.
There was a problem hiding this comment.
There's always at least serial number + error check + stop bit, so it's at least 6. With the field itself, it should be minimum 7. I'm OK with using 7, but 5 doesn't look right, unless I'm missing something.
All that was missing was the information that error check and stop bits were included. With that information, 5 isn't right.
For 0xA0, there's also always Data upload mode and GPS real-time reupload. So if we are including error + stop, its 9, ACC + 8 more, because ACC is still in the buffer when the IF is processed. For the newer version that also has mileage, its >=9
I'll get @Meesen0743 to upload v19 of the documentation so you can see what I mean.
There was a problem hiding this comment.
I looked at the new document and I think my point is still valid. I still don't understand where you're getting 5 from. Why are you talking about excluding error check?
There was a problem hiding this comment.
5 is incorrect. I'm agreeing with you
I'm saying that it should be 7 or higher.
and you are correct, although it should be 9 for that packet. it would be 7 in an alarm packet, if the alarm packet had that data
There was a problem hiding this comment.
knowing that mileage may or may not be included:
1+1+1+2+2+2 = 9
with mileage:
1+1+1+4+2+2+2 = 13
There was a problem hiding this comment.
I'm saying that it should be at least 7, but yes, I think 9 is also safe number.
There was a problem hiding this comment.
I'm saying that it should be at least 7, but yes, I think 9 is also safe number.
i hear you, and i agree. will fix it
There was a problem hiding this comment.
have updated PR, tested working with the trackers we have here
|
Updated Protocol documentation. JM-VL01 GPS Tracker Communication Protocol_V19_20211229 (1).pdf |
Made IF statement more restrictive to prevent accidentally enumerating incorrect data. Now only enumerates ignition state if remaining packet length is either 9 or 13, as per protocol documentation. Tested with multiple JM-VL01 as working with Traccar 4.15
|
Please fix the code style issues. Make sure you run gradle checks locally before pushing the changes. |
Updates Ignition State from 0xA0 location packet (MSG_GPS_2).
This information was previously ignored, and Ignition State was only obtained from 0x13 Heartbeat Packet (MSG_STATUS).
Ignition State is now updated immediately, rather than waiting considerable time for the next Heartbeat, which caused Notifications to be delayed or missed completely.
Note: There are still a few readable byes in the 0xA0 packet that are ignored.