[BUG] Devmod Modules First Index is Always 0
Opened this issue · 5 comments
Describe the bug
Service info sent in message 68 for devmod looks as below.
[false, [["devmod:active", h'f5'], ["devmod:os", h'654c696e7578'], ["devmod:arch", h'65616d643634'], ["devmod:version", h'781a362e362e34312d30333532302d67643364373766313566383432'], ["devmod:device", h'6e46444f2d5072692d446576696365'], ["devmod:sep", h'613a'], ["devmod:bin", h'65616d643634'], ["devmod:nummodules", h'02'], ["devmod:modules", h'830001666465766d6f64'], ["devmod:modules", h'8300016766646f5f737973']]]
There are two "devmod:modules" messages, containing bstr data:
[0, 1, "devmod"]
[0, 1, "fdo_sys"]
The spec is unclear, only stating that "the first element is an integer from zero to devmod:nummodules", but spec authors have verified to me that the purpose of the first element is "start index".
This means that the first message is interpreted as "from index 0, insert 1 element." Therefore the second message should be "from index 1, insert 1 element" and then you have a 2 element list, matching nummodules.
To Reproduce
- Perform TO2 with the demo device.jar
Expected behavior
Modules bstr data should be
[0, 1, "devmod"]
[1, 1, "fdo_sys"]
Example given in the spec is: [uint, uint, tstr1, tstr2, ...]
So you can return the modules one at a time:
...[n, 1, name][n+1, 1, name]...
e.g., [0, 1, "devmod][1, 1, "fdo_sys"]... as you said
or in a larger list
...[10, 1, mod1, mod2, ..., mod10]...
[0, 2, "devmod", "fdo_sys"]
The latter is the original intent, actually.
The reason you don't just list all the modules is that it might not fit into a single message.
So the format gives you a way to describe as many modules as you have room for.
I never intended that you list them one per array, although this is legal.
I also sort of expected that you would list them in some order, but this is not mandated. In fact, you can list them differently on subsequent calls. But the intent is that you assign some order to all your modules, then use this mechanism to chop up the list into chunks that fit into serviceinfo.
I guess the text actually permits you to use the same number for each element, so if you have modules a, b, c, d, you could return
[0, 1, a]
[0, 2, b, c]
[0, 3, d]
and the idea is that you are reordering the list each time. I would not advise this particular behavior, and I'll try to clarify in 1.2.
Here is my proposed change to the text in FDO 1.2:
Enumerates the names of modules supported by this [FIDOIOT] Device.
The device internally assigns an index to each module in the
range [0...nummodules-1]. The list is sent in one or more arrays
containing [index, N, modname1, ... modnameN]. The intent is to send
the full list as a single array, but breaking the list is acceptable,
and may be necessary to allow it to fit into ServiceInfo messages.
Here is my proposed change to the text in FDO 1.2:
Enumerates the names of modules supported by this [FIDOIOT] Device. The device internally assigns an index to each module in the range [0...nummodules-1]. The list is sent in one or more arrays containing [index, N, modname1, ... modnameN]. The intent is to send the full list as a single array, but breaking the list is acceptable, and may be necessary to allow it to fit into ServiceInfo messages.
Only thing I would change is that [index, N, modname1, ...modnameN] is more clear as [i, N, modname(i), ...modname(i+N)] or similar. And maybe there's still a way to make it clear that you shouldn't send any modname(i+N) twice.
Done.