On 14.04.24 06:03, Vincent Mailhol wrote:
This doesn't remove the fact that I think that this naming convention
is stupid because of the RAS syndrome, but I acknowledge that CAN CC
is now the official denomination and thus, that we should adopt it in
our documentation as well.
;-)
Add a space between ISO and the number. Also, update the year:
ISO 15765-2:2024
Interesting! Didn't know there's already a new version.
Will check this out whether we really support ISO 15765-2:2024 ...
Do you have the standard at hand right now or should we leave this as
ISO15765-2:2016 until we know?
I have access to the newer revisions. But I never really invested time
into reading that standard (neither the 2016 nor the 2024 versions).
Regardless, here is a verbatim extract from the Foreworld section of
ISO 15765-2:2024
This fourth edition cancels and replaces the third edition (ISO
15765-2:2016), which has been technically revised.
The main changes are as follows:
- restructured the document to achieve compatibility with OSI
7-layers model;
- introduced T_Data abstract service primitive interface to
achieve compatibility with ISO 14229-2;
- moved all transport layer protocol-related information to Clause 9;
- clarification and editorial corrections
Yes, I've checked the release notes on the ISO website too.
This really looks like editorial stuff that has nothing to do with the
data protocol and its segmentation.
Here, I would suggest the C99 designated field initialization:
struct sockaddr_can addr = {
.can_family = AF_CAN;
.can_ifindex = if_nametoindex("can0");
.tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
.tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
};
Typo in my previous message: the designated initializers are not
separated by colon ";" but by comma ",". So it should have been:
struct sockaddr_can addr = {
.can_family = AF_CAN,
.can_ifindex = if_nametoindex("can0"),
.tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG,
.tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG,
};
Well, this is just a suggestion, feel free to reject it if you do not like it.
At least I don't like it.
These values are usually interactively given on the command line:
> .can_ifindex = if_nametoindex("can0");
> .tp.tx_id = 0x18DA42F1 | CAN_EFF_FLAG;
> .tp.rx_id = 0x18DAF142 | CAN_EFF_FLAG;
So have it in a static field initialization leads to a wrong path IMO.
There is no such limitation that C99 designated initializers should
only work with variables which have static storage duration. In my
suggested example, nothing is static.
I see this as the same thing as below example:
int foo(void);
int bar()
{
int i = foo();
}
int baz()
{
int i;
i = foo();
}
In bar(), the fact that the variable i is initialized at declaration
does not mean that it is static. In both examples, the variable i uses
automatic storage duration.
Here, my preference goes to bar(), but I recognize that baz() is also
perfectly fine. Replace the int type by the struct sockaddr_can type
and the scalar initialization by designated initializers and you
should see the connection.
Oh, sorry. Maybe I expressed myself wrong.
IMHO your way to work with an initializer is correct from the C standpoint.
But I think this is pretty unusual for a code example when an
application programmer starts to work with ISO-TP.
You usually get most of these values from the command line an fill the
struct _by hand_ - and not with a static initialization.
That was my suggestion.
** Different topic **
While replying on this, I encountered something which made me worry a bit:
The type of sockaddr_can.can_ifindex is a signed int:
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/can.h#L243
But if_nametoindex() returns an unsigned int:
https://man7.org/linux/man-pages/man3/if_nametoindex.3.html
Shouldn't sockaddr_can.can_ifindex also be declared as an unsigned int?
The if_index derives from struct netdevice.if_index
https://elixir.bootlin.com/linux/v6.8.6/source/include/linux/netdevice.h#L2158
which is an int.
I don't think this would have an effect in real world to change
sockaddr_can.can_ifindex to an unsigned int.
I wonder if it is more critical to existing user space code to change it
to unsigned int or to leave it as-is ...
Best regards,
Oliver
