Discuss add new device with maintainers (Cradelpoint OBD)
Is the protocol TCP based? If yes, then which frame decoder are you proposing to use? It seems like you are mixing two entirely different things here: protocol decoders and frame decoders.
The protocol can be either TCP or UDP, and I have it currently set to TCP. My suggestion is to create a protocol decoder which extends BaseHttpProtocolDecoder. There is no Frame Decoder involved.
As a side note, I actually experimented with creating my own frame decoder which extends BaseFrameDecoder, but I found just using http was sufficient and also my frame decoder had no easy way to detect the end of the frame as the format of the message is not fixed.
For TCP protocols frame decoder is a requirement.
In my pipeline is HttpRequestDecoder and HttpObjectAggregator, and some other handlers. I assumed that frame decoding was performed by HttpObjectDecoder and thus I do not need to handle this explicitly myself and instead can rely on the Netty framework to do this.
I use io.netty.handler.codec.http.FullHttpRequest to retrieve the frame and then
FullHttpRequest request = (FullHttpRequest) msg;String content = request.content().toString(StandardCharsets.UTF_8);
to retrieve the content, which I parse with the Protocol decoder.
Is it HTTP based protocol? If not, what you are saying doesn't really make sense to me.
Ok, thanks. I likely have this wrong. I wrote the code assuming it was HTTP, but now I think it is probably just TCP or UDP (if selected). I will have access to the device this weekend and will check it.
On a positive note, I actually wrote a simple Frame Decoder not based on HTTP initially, and it just returnsbuf.readRetainedSlice(buf.readableBytes())
Let me experiment a little further. Thanks for pointing a possible problem with my assumption on the transport layer.
Anton,
I have confirmed that the device sends TCP. So, based on this the proposal now is as follows:
Create a new pipeline class for Cradlepoint OBD device. The pipeline will be something like
protected void addProtocolHandlers(PipelineBuilder pipeline) { pipeline.addLast(new CradlePointOBDFrameDecoder()); pipeline.addLast(new StringDecoder()); pipeline.addLast(new StringEncoder()); pipeline.addLast(new CradlePointOBDProtocolDecoder(CradlepointOBDProtocol.this));Create a new protocol decoder class to parse the various Cradlepoint sentences including the typical NEMA based GPS sentences but also the new OBD sentence.
Create a frame decoder class to read the incoming byte stream and pass the various sentences in one frame to the protocol decoder. Unfortunately it is not possible to use an existing frame decoder like LineBasedFrameDecoder.
Update defaults.xml with a proposed port for the new device
If the above is the right direction, I will continue to test what I have already written and will do a pull request only after field testing to make sure it all works. Look forward to feedback.
What would CradlePointOBDFrameDecoder look like?
Anton,
The decoder implementation would be guided by the suggesiton from here.
There is an interesting situation in the Cradlepoint device. Which sentences are presented is under the control of the User of the device. These sentences are turned on and off in the device's configuration. These sentences are not fixed length, but they are always terminated by a special character ("*") to indicate a checksum which is then followed by a carriage return and line feed. In addition, one of the sentences contains a set of key-value pairs, and the User can decide which key-value pairs are presented. These key-value-pairs are individually enabled or not in the device's configuration. Again this sentence is terminated by a checksum and carriage return linefeed. The combination of these conditions, means the definition of the frame is little ill-defined.
My current thought is that the decoder would split the received ByteBufs on line endings ("\r\n") combined with a uniqueness check on sentence identifiers or headers (e.g., cannot contain more than one $GPRMC sentence in a single frame). That is, a frame may contain certain sentences, but only one instance of any sentence. It is unfortunate that the frame decoder needs to rely on the underlying protocol, but I can't think of a more robust way to do this.
At present I have written a simple frame decoder which just assumes the byte buffer is a frame. It works in the real world, at least good enough for testing the protocol decoder and protocol pipeline, but not good enough for real code.
Hope the above makes sense.
I don't see any correct way to implement what you want to implement, unless we ask user to provide extra configuration for the frame decoder.
Oh, one more thing. The order of the sentences within the frame is fixed. That is, $GPGGA is before $GPVTG which is before $GPRMC which is before $PCPTOBD. Order is maintained even if some sentences are not present.
The User can only control the configuration of what is and what is not presented. The manufacture of the device implements the protocol and this protocol is not under user control. The manufacturer is basically following the NEMA 0183 standard, including the use of $P to proceed proprietary sentences. The standard basically anticipates that manufacturers will make extensions.
I agree, the logical frame should be a sentence, but this causes issues with traccar, as there seems to be a hard one-to-one relationship in traccar between frames and decoded position objects. I would much prefer to use LineBasedFrameDecoder from Netty and somehow combine the frames elsewhere. But I am not sure that this is possible in traccar.
If there is no way to tell where a set of messages starts and end, then I don't see any way to implement what you want to implement. Assuming that incoming buffer contains full set is not a good solution.
Anton,
Ok. I agree that assuming that incoming buffer contains full set is not a good solution. That is why I said "...but not good enough for real code".
So, if we agree that frames should be based on sentences, then I will go back to LineBasedFrameDecoder. I actually like this idea as the NEMA 0183 protocol is fundamentally based on sentences ending in /r/n and the "frame" checksum. So, now I am forced to solve this problem of multiple sentences in the Protocol Decoder?
I looked hard at the T55 protocol decoder class, and it's approach to retrieve the last position, is not very elegant. Basically the T55 protocol suffers the same problem, but solves it in a very strange way (in my opinion). Hope you have a good suggestion in a nice way.
The contributing guidelines for pull requests recommend that I discuss my proposed solution with the maintainers. This thread is intended for this discussion.
Background to Pull Request
I would like to support the Cradlepoint series of routers which provide support for GPS tracking as well as for on-board diagnostics (aka OBD) from vehicles. The protocol used by this device is a variant and extension of NEMA 0183 sentences. Issue #3987 already provides a description of the protocol. The most important aspect of the protocol is the addition of a new sentence which is proprietary to the Cradlepoint product and has the identifier $PCPTOBD. This sentence provides all OBD data but contains no GPS fix information.
Proposed Solution
The current LineBasedFrameDecoder appears to be a poor choice for this protocol, as the new $PCPTOBD line does not contain any GPS fix details, and use of this Frame decoder would require the previous GPS fix to be retrieved and used. This would also create a second position record.
Instead I propose that I create a new Protocol Decoder based on BaseHttpProtocolDecoder which will pass all of the NEMA sentences on one message to the new Protocol Decoder. If the $GPRMC sentence is present in the message, this new protocol decoder will use the information contained in the $GPRMC sentence (which stands for Recommend Minimum) to obtain the time and date of the fix was taken, if the fix is valid or not, the latitude and longitude of the fix, the velocity and the track or course. The magnetic variation and checksum will be ignored from $GPRMC sentence. If the $GPGGA sentence is also present in the message, then fix quality, number of satellites and altitude will be added to position object. Time of fix, latitude, longitude, horizontal dilution, height of geoid and checksum will be ignored from $GPGGA. If $GPGGA is present but $GPRMC is not, then no record will be creates as the date of the fix cannot be determined. The $GPVTG sentence will not be supported and not parsed even if it is in the message.
The $PCPTOBD will be supported with OBD data added to the position object as attribute information. There are existing OBD fields that will be reused, but will need to introduce new fields as the Cradlepoint OBD data is more extensive than traccar presently supports. The details of how to add these can be discussed in the pull request, but initial assumptions and early testing indicates that no changes to the position model are required. In the $PCPTOBD sentence format, none of the fields are required to be present, so the code will be flexible to process only the data that is presented. And even when data is presented (field is enabled) the value can be null. This will all be handled gracefully by the code and will not assume a fixed order of the fields. All presented data without null (!.isEmpty) will be process except the checksum.
The T55 protocol handles NEMA 0183, but will not be extended as it is based on the LineBasedFrameDecoder and this presents issues as mentioned prior. Instead a new protocol will be created. The working name and java class for this new protocol is currently CradlepointOBDhttpProtocolDecoder.java. There will also obviously be a CradlepointOBDhttpProtocol.java file. I will send http responses back, either .OK or .BAD_REQUEST as appropriate. I will propose the next available port in defaults.xml for this new protocol. The pull request will include test classes. I would also write a description on how to use to configure the Cradlepoint to work with traccar (although I am not sure where to submit this description).
Comments Please
Please let me know if the above is the best way forward, or else indicate a different architectural direction. I have written the code and will be field testing over the next 2 to 6 weeks before submitting the pull request.