nasa
JSON code generation for dictionary
| F´ Version | |
| Affected Component |
Feature Description
Generate JSON from the dictionary data structure. The code generation should follow the specification given here: https://github.com/nasa/fprime/blob/devel/docs/Design/fpp-json-dict.md.
Copied from https://github.com/fprime-community/fpp/issues/377
@jwest115 rather than creating a new issue I decided it would be useful to simply put some comments here for you and others to consider. I was looking at the dictionary spec in https://github.com/jwest115/fprime/blob/fpp-dict-json-spec/docs/Design/fpp-json-dict.md#type-names when I wrote these comments. At the end of the document the example dictionary looks pretty much like what I expected. It would be nice to have some words in the specification to clarify ideas and most importantly motivations.
Some of the comments might be just because I do not quite understand all that you are doing so let me know where I am totally off target. These are intended just preliminary since I did not look that closely at all the details.
Also I am assuming this dictionary format is not intended to be general but rather very F´and F´´ centric. That being the case I think there could room for simplifications. I would encourage that.
- For primitive types there seems to be quite a bit of redundant information. For example a simple integer is
{
"name": "U8",
"kind": "integer",
"size": 8,
"signed": false,
}
where I think it could be just as useful to use something like
{
"name":"U8"
}
better yet I think what you might want is simply something with
{
"name": <this is some variable identifier name>
"kind": <this can be U8, U16, U32, U64, I8, I16, I32, I64, F32, F64, BOOL, STRING, ARRAY, ENUM types>
"size": <this is an optional key used only for STRING, ARRAY, and maybe STRUCT kind>
etc. etc. other optional keys as you specify depending on the kind.
}
Note the <...> express a general idea of what goes there. The idea is to eliminate redundant information and try to minimize the keys used.
- I did not understand the qualified identify?
- I did understand your values section that much. Particularly I would think a kind key would never be allowed to be invalid because this implies your generating bad dictionary data and I would think you never want to allow this.
- I like the idea of having potential word values for certain telemetry that is "null", "infinite", "negativeinfinite" this is an improvement over the way I have used this stuff. Perhaps the telemetry component has evolved to support these things.
- Probably missing some Fpp details but I am not sure what the difference between Formal Parameters and Parameters are other then the string name?? Perhaps add words.
- For Parameters you have something that looks like this
{
"identifier": "MyModule.FirstComponent.Parameter1",
"description": "This is the annotation for Parameter 1",
"type": {
"name": "U32",
"kind": "integer",
"signed": false,
"size": 32
},
"default": 0,
"numericIdentifier": "256"
}
Perhaps this might be simpler and a bit more consistent with above if it looked like this
{
"identifier": "MyModule.FirstComponent.Parameter1",
"name": "A physical name given so systems engineers know what this is",
"description": "This is the annotation for Parameter 1",
"kind": "U32",
"default": 0,
"id": 256
}
I think adding a new "name" key would be very useful so that the dictionaries can present with both logical code name as an identifier but also have a more meaningful physical name that an engineering looking at a display who is not familiar with the code would understand. This would be good to have for commands and telemetry items as well.
I think the numeric identifier can be shortened to "id" like our current dictionaries. Also the id's are always numbers so make it numeric.
- The commands look good but again I prefer a little shorter names and simpler structure. You have
{
"identifier": "MyComponents.FirstComponent.SyncParams",
"commandKind": "sync",
"opcode": 256,
"description": "A sync command with parameters",
"formalParams": [
{
"identifier": "param1",
"description": "Param 1",
"type": {
"name": "U32",
"kind": "integer",
"size": 32,
"signed": false,
},
"ref": false
},
{
"identifier": "param2",
"description": "Param 2",
"type": {
"name": "string",
"kind": "string",
"size": ""
},
"ref": false
}
],
}
how about this
{
"identifier": "MyComponents.FirstComponent.SyncParams",
"name"" "SetSyncParams"
"kind": "sync",
"opcode": 256,
"description": "A sync command with parameters",
"args": [
{
"name": "param1",
"description": "Param 1",
"kind": "U32",
"ref": false
},
{
"name": "param2",
"description": "Param 2",
"kind": "string",
"size": 40,
"ref": false
}
],
}
Note the "commandKind" is now just "kind" but in a different context. The "type" is now just "kind" again. Attempting to reduce the number of different keys that exist would be good.
- For the telemetry I would try and do something similar to above so I will not repeat the example. I will comment on the "limit" key since I see you using colors in there that are limited to green, yellow and red but split up. I want to give you two alternatives. The first is to not use any colors but leave that as a ground system tweak sort of thing. So the "limit" key would now becomes an array of {"min": ..., "max":...} elements. The second form which I sort of prefer is
{ "limits" : [
{"green" : [{"min": -1},{"max":1}]},
{"yellow" : [{"min": -2},{"max":2}]},
{"red" : [{"min": -3},{"max":3}]},
]}
In fact you do not need to limit this to just three colors since you could have like even 4 or 6 colored ranges. Typically it is just three colors but being general in this case does not cost much and might be useful.
- Some sort of ideas apply to events. Unfortunately to me events always have looked incredibly ugly in F´ and I wish they could take more of a C++ or C style of thing. Just my opinion.
- For the Data Product Records I will not comment much but say that I think you want something to describe the structure of a single record type and then define how it gets replicated when serialized. Data Products have thousands of records so defining each in a dictionary is not practical.
- I did not understand the Container. Perhaps not fully defined yet. Perhaps containers contain recorders of the data products.
- For the Metadata you have lots of great version information. I would suggest including the date the dictionary was generated and the user name or who or what account generated it. These are very useful during development and testing to just know. Perhaps you might also want to indicate in Metadata if opcodes are in hex or dec or will everything be always in dec.
I like the basic look of the example dictionary at the end. Hope this comments are helpful and create some ideas and thoughts. I will try and make your 1 Feb presentation but my schedule gets tight. If you like me to test something let me know since I can do this on my personnel Linux machine in the evenings very easily.
Thanks @ljreder for the feedback! Here are some comments:
- This is something we also thought about and decided it was good to go the more "verbose" route since it is inline with other JSON specifications.
- The qualified identifier type name is used to refer to a enum, struct, or array definition. For example, an array A in module M would have qualified identifier M.A
3. I used the "invalid" kind to represent the kind of null, infinity, and negative values - but definitely open to feedback if you think this could be improved.
5. Formal parameters refer to parameters that are supplied to a command (see below example) and are different then parameters. In the 1st example - param1 and param2 are formal parameters (args that can be supplied to the command), while in the 2nd example - Parameter1 is a parameter.
@ A sync command with parameters
sync command Command1(
param1: U32 @< Param 1
param2: string @< Param 2
) opcode 0x100
@ Parameter 1
param Parameter1: U32 \
id 0x100 \
set opcode 0x101 \
save opcode 0x102
6 & 7. Maybe the "name" information could be supplied as part of the description (ie: add within an annotation in the FPP model)? For the shortened version vs. current version, similar to 1. we went the more verbose route. 8. The red, orange, and yellow colors are defined as part of the FPP spec, but I see what you are saying about min/max. 12. In response to hex or decimal opcodes - we went the route of requiring opcodes to be in decimal every time. I also think date is something we could add to the dictionary if that info is useful.
Work to go:
- Finish adding in optional fields
- Need to merge in data product branch of FPP and add record and container JSON generation
- Add in fully qualified names for commands, events, channels, parameters, records, and containers
- Add in dictionary metadata JSON - deployment name, framework version, project version, library versions, dictionary spec version
@jwest115 glad you found the comments useful. Here are a few follow on comments about your above feedback.
- Going the more verbose approach is fine, but having redundancy that could potentially get out of sync is not a good idea. You could simply remove the name attribute and now your more verbose. Or just leave it as is as well since it is no big deal. I will say for a human reading the dictionary it would be much simpler not to have the extraneous stuff and extra object resulting.
- Yes I see what your doing. From a systems point of view the software modules really should not even appear in the dictionary but it is useful never the less.
- Again I would think the kind key would always be something valid and never invalid or null. But I am not looking at the spec now and I think you may be using the kind key to mean different things at different times. Hopefully all the keys are used with consistent meanings.
- Using the term formal parameter for what are simply command arguments is kind bad terminology. Why not just call them command arguments or just args as we have done for years. I suppose it was clear to everybody from context though.
- & 7. Yes you could do something with annotations. I will leave it to you guys to dream this up if you like it. I would encourage a uniquely new thing for this though since your including software names as qualified identifiers.
- OMG ground system colors restricted in a language specification. Oh well. But nice if for each color the min/max are grouped.
- YES thank you for restricting opcodes to all decimal. On my past projects using F prime the mixture people have done was driving me crazy. Hopefully all the ID's for events, channels, parameters, etc. are decimal as well I assume.
Update:
- Almost done adding in optional fields, just need telemetry channel limits. Updated spec to reflect changes: https://github.com/jwest115/fprime/commit/ab686ca67432725ba8d1cb3b9592df636dc26797
- Merged in branch that has DPs, added records and collections to dictionary data structure and JSON generation
- Add in qualified names for commands, events, channels, parameters, records, and containers - after speaking to Rob, I'll need to adjust the dictionary data structure a bit to include component instances, plan to update it this week
- Still need to add in dictionary metadata JSON - deployment name, framework version, project version, library versions, dictionary spec version
Work to go:
- Need to add in dictionary metadata JSON - deployment name, framework version, project version, library versions, dictionary spec version
- Update spec / JSON fields based on feedback
Added in JSON for:
- deployment name
- framework version
- project version
- library versions
- dictionary spec version
Added verbose option so that certain fields (ie: command priority and queue full behavior) can be excluded/included in the dictionary Updated spec based on feedback, will probably PR to fprime devel soon
Feedback/to do:
- remove verbose option
- update spec and JSON generator to have “typeDefinitions” instead of "arrays", "enums", and "structs" - put all these under "typeDefintions"
Removed verbose option, updated spec. Will move this into review today/tomorrow and will link PR here