Re: [Interest] QCanBusDevice inheritance

[Blasche Alexander]

>> Another inefficiency of the QIODevice approach would be that each frame
Hi,

>> would have to be serialized to a QByteArray on the user API side just to be
> >unserialized on the backend side again. I have yet to see a backend API that
> >doesn't deal with a frame based API. There is simply nothing gained.
>
>I don't get why each frame would have to be serialized if
>QCanBusDevice inherits QIODevice. The implementation of
>QCanBusDevice::writeFrame() would not change. It would, on the other
>hand, require to implement QIODevice::writeData() which would either
>call QCanBusDevice::writeFrame() or directly the backend API
>effectively doing some "unserialization".

The question is how you get to the serialized QCanBusFrame in the first place. 

Based on your suggestion above the usual order of calls for a QIODevice based 
API across the stack is such that

const char* serialized_frame = ...
device->write(serialized_frame;
 -> device->writeData(serialized_frame, size); // deserializes serialized_frame
-> device->writeFrame(frame) // based on your suggestion above

How did the user create serialized_frame if not using some QDataStream operator 
that created it from QCanBusFrame? Surely you cannot expect the API user to 
create the data stream representing a frame from scratch but via QCanBusFrame?

I am sorry all I see is artificially passing everything through a serialization 
process where the top and the bottom of the call series end up with the same 
QCanBusFrame. Then I may as well permit the user to directly call writeFrame(). 
And that's what we did.

Let me ask you this, why do you want it to be a QIODevice? It's not because the 
Qt docs stating "The QIODevice class is the base interface class of all I/O 
devices in Qt", is it? For me the API just doesn't fit and that's reason enough 
for me to contradict the QIODevice documentation ;)

--
Alex



___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

Re: [Interest] QCanBusDevice inheritance

[Benjamin TERRIER]

2015-09-10 9:49 GMT+02:00 Blasche Alexander
:
>
>>> Another inefficiency of the QIODevice approach would be that each frame
> Hi,
>
>>> would have to be serialized to a QByteArray on the user API side just to be
>> >unserialized on the backend side again. I have yet to see a backend API that
>> >doesn't deal with a frame based API. There is simply nothing gained.
>>
>>I don't get why each frame would have to be serialized if
>>QCanBusDevice inherits QIODevice. The implementation of
>>QCanBusDevice::writeFrame() would not change. It would, on the other
>>hand, require to implement QIODevice::writeData() which would either
>>call QCanBusDevice::writeFrame() or directly the backend API
>>effectively doing some "unserialization".
>
> The question is how you get to the serialized QCanBusFrame in the first place.
>
> Based on your suggestion above the usual order of calls for a QIODevice based 
> API across the stack is such that
>
> const char* serialized_frame = ...
> device->write(serialized_frame;
>  -> device->writeData(serialized_frame, size); // deserializes 
> serialized_frame
> -> device->writeFrame(frame) // based on your suggestion above
>
> How did the user create serialized_frame if not using some QDataStream 
> operator that created it from QCanBusFrame? Surely you cannot expect the API 
> user to create the data stream representing a frame from scratch but via 
> QCanBusFrame?
>
> I am sorry all I see is artificially passing everything through a 
> serialization process where the top and the bottom of the call series end up 
> with the same QCanBusFrame. Then I may as well permit the user to directly 
> call writeFrame(). And that's what we did.
>
> Let me ask you this, why do you want it to be a QIODevice? It's not because 
> the Qt docs stating "The QIODevice class is the base interface class of all 
> I/O devices in Qt", is it? For me the API just doesn't fit and that's reason 
> enough for me to contradict the QIODevice documentation ;)
>
> --
> Alex

I do not object that the user can directly call writeFrame(). And I
agree that using the QIODevice API in such case would lead to an
unnecessary overload.

Nonetheless, I think that, as it has been done with QUdpSocket, adding
the QIODevice API to the existing CAN API would only make the Qt
framework more coherent. (It is just a matter of taste and policy.)

To be clear, I don't want the QCanBusDevice API to fit inside the
QIODevice API, but I would find it more coherent if QCanBusDevice
would offer the QIODevice API as a fallback ; the current CAN API
would still be public and the recommended way to use QCanBusDevice.

Benjamin
___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

Re: [Interest] QCanBusDevice inheritance

[Denis Shienkov]

Benjamin,

I forwarded your message to the mailing list back (you likely forgot to do
it?). Now it could available to comments and for other developers (if you
don't object).

BR,
Denis

2015-09-09 15:46 GMT+03:00 Denis Shienkov :

> Benjamin,
>
> I forwarded your message to the mailing list back (you likely forgot to do
> it?). Now it could available to comments and for other developers (if you
> don't object).
>
> 2015-09-09 11:10 GMT+03:00 Benjamin TERRIER :
>
>> 2015-09-08 19:37 GMT+02:00 Denis Shienkov :
>> > Hi.
>> >
>> > Because QIODevice is inconsistent with the CAN frames. The CAN uses a
>> > minimal entity - frame, which can not be divided to bytes. Besides,
>> > QIODevice's read/write/bytesAvailable methods has not sense, you can not
>> > read/write a half of frame and so on. You can not know a size of frame
>> > to serialize it, because its size is various. And others, others, others
>> > CAN specific reasons. Please, read comments in codereview commits on
>> > gerrit for acquaintance.
>> >
>> > BR,
>> > Denis
>> >
>> > 08.09.2015 18:50, Benjamin TERRIER пишет:
>> >> Hi,
>> >>
>> >> Following the announce of Qt 5.6 Alpha, I checked the brand new
>> >> QCanBusDevice and I've been wondering why the developers did not want
>> >> to use QIODevice (the git logs show that all dependencies have been
>> >> purposely removed).
>> >>
>> >> Cheers,
>> >>
>> >> Benjamin
>> >> ___
>> >> Interest mailing list
>> >> Interest@qt-project.org
>> >> http://lists.qt-project.org/mailman/listinfo/interest
>> >
>> > ___
>> > Interest mailing list
>> > Interest@qt-project.org
>> > http://lists.qt-project.org/mailman/listinfo/interest
>>
>> Hi,
>>
>> Thanks for your response but I still don't get why QIODevice has been
>> discarded for CAN while it is used for QUdpSocket.
>>
>> First, frames can always be divided into bytes, there length is an
>> integer multiple of byte. If you are talking about the fact that
>> fields within a frame are not 8 bits longs (identifier can be 11),
>> this should be handled by the underlying CAN library.
>>
>> I also don't get why not knowing the size of a frame can be a problem,
>> UDP datagrams size can vary yet QUdpSocket inherits QIODevice.
>>
>> Finally, QIODevice read/write/bytesAvailable can be given a sense, for
>> instance bytesAvailable could return the sum of the size of all
>> received CAN frames (I do agree it has a limited use but it does not
>> mean it doesn't make sense).
>>
>> My point is a QCanBusDevice is an I/O device and therefore should be a
>> QIODevice as, I quote, "The QIODevice class is the base interface
>> class of all I/O devices in Qt". And the fact that some QIODevice
>> functions are not a perfect fit for CAN should not be a reason not to
>> use it as such cases seems to have been taken into account in the
>> design of the QIODevice class (look at QIODevice::pos(), it is
>> explicitly stated that this function might not make sense).
>>
>> BR,
>>
>> Benjamin
>>
>
>
___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

Re: [Interest] QCanBusDevice inheritance

[Blasche Alexander]

Hi,


>Thanks for your response but I still don't get why QIODevice has been
>discarded for CAN while it is used for QUdpSocket.

>First, frames can always be divided into bytes, there length is an
>integer multiple of byte. If you are talking about the fact that
>fields within a frame are not 8 bits longs (identifier can be 11),
>this should be handled by the underlying CAN library.

The QIODevice architecture was not a good fit for CAN bus. The API must enable 
the user to interpret the various frames received on the bus. In addition to 
the payload, each frame may come from different sources, has different types 
and other attributes. This means that the API requires at least an abstraction 
on frame level. For each new frame that you send you may adjust the frame 
attributes as well.

This is considerably different from a QIODevice based architecture where you 
generally open a connection to a remote entity and once the connection is 
established you merely keep exchanging payload. After all you don't set a new 
IP for each UDP datagram. There is no requirement to dissect the incoming data 
stream on the backend either. We generally copy the raw data to a socket.

Another inefficiency of the QIODevice approach would be that each frame would 
have to be serialized to a QByteArray on the user API side just to be 
unserialized on the backend side again. I have yet to see a backend API that 
doesn't deal with a frame based API. There is simply nothing gained.

Another argument, the frame based API serves as an input gate. We can make 
assumptions about data format to be send by the API user as the user has to fit 
it into a QCanBusFrame. How should a backend react to an arbitrary data stream 
coming from the user? We have to find all the frame details in it before we 
send it further on. Hence a QIODevice API invites "frame input errors" which is 
not very user friendly.

I hope this helps to understand the reasons for the change.

--
Alex
___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

Re: [Interest] QCanBusDevice inheritance

[Benjamin TERRIER]

2015-09-09 15:18 GMT+02:00 Blasche Alexander
:
> Hi,
>
>
>>Thanks for your response but I still don't get why QIODevice has been
>>discarded for CAN while it is used for QUdpSocket.
>
>>First, frames can always be divided into bytes, there length is an
>>integer multiple of byte. If you are talking about the fact that
>>fields within a frame are not 8 bits longs (identifier can be 11),
>>this should be handled by the underlying CAN library.
>
> The QIODevice architecture was not a good fit for CAN bus. The API must
> enable the user to interpret the various frames received on the bus. In
> addition to the payload, each frame may come from different sources, has
> different types and other attributes. This means that the API requires at
> least an abstraction on frame level. For each new frame that you send you
> may adjust the frame attributes as well.

Yes, CAN requires an abstraction on frame level.

> This is considerably different from a QIODevice based architecture where you
> generally open a connection to a remote entity and once the connection is
> established you merely keep exchanging payload. After all you don't set a
> new IP for each UDP datagram. There is no requirement to dissect the
> incoming data stream on the backend either. We generally copy the raw data
> to a socket.

Check:
  qint64 QUdpSocket::writeDatagram(const QByteArray & datagram, const
QHostAddress & host, quint16 port)

You do set a new IP for each UDP datagram.
And as for QUdpSocket you could have enabled a virtual connection
behavior (see last line of QUdpSocket detailed description in Qt
documentation).


>
> Another inefficiency of the QIODevice approach would be that each frame
> would have to be serialized to a QByteArray on the user API side just to be
> unserialized on the backend side again. I have yet to see a backend API that
> doesn't deal with a frame based API. There is simply nothing gained.

I don't get why each frame would have to be serialized if
QCanBusDevice inherits QIODevice. The implementation of
QCanBusDevice::writeFrame() would not change. It would, on the other
hand, require to implement QIODevice::writeData() which would either
call QCanBusDevice::writeFrame() or directly the backend API
effectively doing some "unserialization".



> Another argument, the frame based API serves as an input gate. We can make
> assumptions about data format to be send by the API user as the user has to
> fit it into a QCanBusFrame. How should a backend react to an arbitrary data
> stream coming from the user? We have to find all the frame details in it
> before we send it further on. Hence a QIODevice API invites "frame input
> errors" which is not very user friendly.
>
> I hope this helps to understand the reasons for the change.
>
> --
> Alex
>
> ___
> Interest mailing list
> Interest@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/interest
>

I do agree that the QIODevice API is not a perfect fit for a CAN API
and that it is not user friendly. However, I do think it would have
been better to keep the inheritance using a frame level API as the
main entry point for QCanBusDevice, and the QIODevice API as a
fallback which would rely on the frame level API.
I think it would have been more coherent with the rest of the Qt
framework, particularly when looking at QUdpSocket.

BR,

Benjamin
___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

Re: [Interest] QCanBusDevice inheritance

[Thiago Macieira]

On Wednesday 09 September 2015 13:18:33 Blasche Alexander wrote:
> The QIODevice architecture was not a good fit for CAN bus. The API must
> enable the user to interpret the various frames received on the bus. In
> addition to the payload, each frame may come from different sources, has
> different types and other attributes. This means that the API requires at
> least an abstraction on frame level. For each new frame that you send you
> may adjust the frame attributes as well.

You're describing QUdpDatagram (https://codereview.qt-project.org/108336), 
which is being added to QUdpSocket.

> After all you don't set a new IP for each UDP datagram. 

You can do it. You don't have to, but you can.

That said, the send/receive datagram functionality bypasses the regular 
QIODevice virtuals and goes straight to the socket engine.
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

[Interest] QCanBusDevice inheritance

[Benjamin TERRIER]

Hi,

Following the announce of Qt 5.6 Alpha, I checked the brand new
QCanBusDevice and I've been wondering why the developers did not want
to use QIODevice (the git logs show that all dependencies have been
purposely removed).

Cheers,

Benjamin
___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest

Re: [Interest] QCanBusDevice inheritance

[Denis Shienkov]

Hi.

Because QIODevice is inconsistent with the CAN frames. The CAN uses a 
minimal entity - frame, which can not be divided to bytes. Besides, 
QIODevice's read/write/bytesAvailable methods has not sense, you can not 
read/write a half of frame and so on. You can not know a size of frame 
to serialize it, because its size is various. And others, others, others 
CAN specific reasons. Please, read comments in codereview commits on 
gerrit for acquaintance.

BR,
Denis

08.09.2015 18:50, Benjamin TERRIER пишет:
> Hi,
>
> Following the announce of Qt 5.6 Alpha, I checked the brand new
> QCanBusDevice and I've been wondering why the developers did not want
> to use QIODevice (the git logs show that all dependencies have been
> purposely removed).
>
> Cheers,
>
> Benjamin
> ___
> Interest mailing list
> Interest@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/interest

___
Interest mailing list
Interest@qt-project.org
http://lists.qt-project.org/mailman/listinfo/interest