Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change NID db format #83

Closed
4 tasks done
yifanlu opened this issue Nov 11, 2016 · 37 comments
Closed
4 tasks done

Change NID db format #83

yifanlu opened this issue Nov 11, 2016 · 37 comments
Assignees
Labels

Comments

@yifanlu
Copy link
Contributor

yifanlu commented Nov 11, 2016

@yifanlu
Copy link
Contributor Author

yifanlu commented Nov 11, 2016

We should also have some sort of indicator for the new version. Maybe a tag

// version: 2

at the start? But then it won't comply with JSON standard(s). It's kinda messy to add a new field/object though.

@devnoname120
Copy link
Member

About 2):

  • Advantages of hexadecimal:
    • Consistent with the PSP NIDs.
    • Some NID lists on the Internet are in hexadecimal, so Sony probably uses it too.
    • 0xFFFFFFFF is a more obvious maximum value than 4294967295.
  • Advantages of decimal:
    • Easier parsing and comparing, especially in Python
    • Stay consistent with the previous version

@devnoname120
Copy link
Member

devnoname120 commented Nov 11, 2016

We should also have some sort of indicator for the new version. Maybe a tag
// version: 2
at the start?

JSON doesn't support comments, so a tag version: 2 at the root of the JSON would work better. It would also allow to programmatically find the version.

@d3m3vilurr
Copy link
Contributor

@devnoname120 parse hex also easy in python, but json not support hexdeciaml.

@frangarcj
Copy link
Contributor

I'm testing hex right now
I agree with version: 2

@d3m3vilurr
Copy link
Contributor

i mean json spec. they not support hex number. if can work, probably it's non standard lib extension.

@frangarcj
Copy link
Contributor

@frangarcj
Copy link
Contributor

Confirmed working with hex values. should I change format?

@devnoname120
Copy link
Member

devnoname120 commented Nov 13, 2016

We need to decide whether we want to switch to YAML or not.
An advantage of YAML is that it natively supports hex values. I'm not sure how well YAML libraries handle it though.

Here is how it would look: https://gist.github.com/devnoname120/ff2a08dbcfa0126d9099ce894f7fe783
It seems more readable to me.

@frangarcj
Copy link
Contributor

I think json with hex values is the best solution:

  • It's already developed
  • You can easily export fron json to other formats if needed (eg. using nodejs)

If you like the idea I can make a pull request with latest db.json in new format
Changes in vita-import-load are already done

@d3m3vilurr
Copy link
Contributor

yeah.. i'm ok at this time.

but i want to change other format, too.
hex string method looks tricky.

@yifanlu yifanlu modified the milestone: taiHENkaku Release Nov 17, 2016
@DaveeFTW
Copy link
Contributor

I'm happy to go with hex strings if we enforce that strings must be 10 characters long, beginning with "0x", followed by eight hexadecimal characters where all alphabet characters are capitalised.

Additionally, we should have a simple validator script to automatically verify this repo's json db.

@devnoname120
Copy link
Member

I agree with Davee.

The validator script should be very easy to do in Python.

@yne
Copy link
Contributor

yne commented Nov 20, 2016

Easy indeed, we simply need to define it schema, then use it to validate.

@d3m3vilurr
Copy link
Contributor

i didn't like json schema (it look too ugly format. IMO). probably, json schema can check formatting using regex.
but in this case, also we can write plain python script using just json module.
that module is already standard library, so not need extra dependencies for validation.
just need dict iterations..
(yeah it can make validation script to be little mess, it just pros & cons)

@jdek
Copy link
Contributor

jdek commented Nov 21, 2016

If we're changing the JSON format, can we use something which is easily parseable in all languages, i.e. something which doesn't require the JSON parser to use objects as arrays and have a custom object for each item in the array.

{ "version" : "3.61",
  "modules" : [
    {
      "name" : "SceDeci4p",
      "nid" : "0x27",
      "libraries" : [
        {
          "name" : "SceDeci4pUserp",
          "nid" : "0x797E39C0",
          "kernel" : false,
          "functions" : [
              { "name" : "sceKernelDeci4pClose", "nid" : "0x63B0C50F" },
              { "name" : "sceKernelDeci4pDisableWatchpoint", "nid" : "0x5A4CDF97" },
              { "name" : "sceKernelDeci4pEnableWatchpoint", "nid" : "0x3BC66BD8" },
              { "name" : "sceKernelDeci4pIsProcessAttached", "nid" : "0xC4E1D86D" },
              { "name" : "sceKernelDeci4pOpen", "nid" : "0x28578FE8" },
              { "name" : "sceKernelDeci4pRead", "nid" : "0x971E1C66" },
              { "name" : "sceKernelDeci4pRegisterCallback", "nid" : "0x73371F35" },
              { "name" : "sceKernelDeci4pWrite", "nid" : "0xCDA3AAAC" },
          ],
          "variables": []
        }
      ]
    },
  ]
}

I like the look of YAML though.

@frangarcj
Copy link
Contributor

I like that json version better

El lun., 21 nov. 2016 a las 9:29, Josh de Kock (notifications@github.com)
escribió:

If we're changing the JSON format, can we use something which is easily
parseable in all languages, i.e. something which doesn't require the JSON
parser to use objects as arrays and have a custom object for each item in
the array.

{ "version" : "3.61",
"modules" : [
{
"name" : "SceDeci4p",
"nid" : "0x27",
"libraries" : [
{
"name" : "SceDeci4pUserp",
"nid" : "0x797E39C0",
"kernel" : false,
"functions" : [
{ "name" : "sceKernelDeci4pClose", "nid" : "0x63B0C50F" },
{ "name" : "sceKernelDeci4pDisableWatchpoint", "nid" : "0x5A4CDF97" },
{ "name" : "sceKernelDeci4pEnableWatchpoint", "nid" : "0x3BC66BD8" },
{ "name" : "sceKernelDeci4pIsProcessAttached", "nid" : "0xC4E1D86D" },
{ "name" : "sceKernelDeci4pOpen", "nid" : "0x28578FE8" },
{ "name" : "sceKernelDeci4pRead", "nid" : "0x971E1C66" },
{ "name" : "sceKernelDeci4pRegisterCallback", "nid" : "0x73371F35" },
{ "name" : "sceKernelDeci4pWrite", "nid" : "0xCDA3AAAC" },
],
"variables": []
}
]
},
]
}

I like the look of YAML though.


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#83 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAYaFnPWEzi_7aL3Sf4jOvIuG5-WxCLhks5rAVZWgaJpZM4KwFST
.

@devnoname120 devnoname120 mentioned this issue Nov 21, 2016
3 tasks
@devnoname120
Copy link
Member

@enoposix I think that @yifanlu wanted to use version for the DB specification version. Should we also have a field for the firmware version?

@jdek
Copy link
Contributor

jdek commented Nov 21, 2016

@yifanlu sure, well you could just add a "firmware" field below version easily. I still think the YAML is a better idea--if we are going to be doing a fairly drastic change anyway, the YAML is a lot cleaner. It can be generated extremely easy without the use of a library (just one of the many benefits).

@devnoname120
Copy link
Member

devnoname120 commented Nov 21, 2016

@enoposix The format has already been discussed. Most of us prefer YAML for its cleanness, but it's harder to parse because it has complicated features that we don't need (e.g. references, data-type casting, etc.). Since this file is more meant to be read by a program, JSON seemed fine.

@jdek
Copy link
Contributor

jdek commented Nov 21, 2016

@devnoname120 right, sure. I guess that makes sense.

Why not go all out and allow multiple firmwares in one DB then?

{
  "version" : 2,
  "firmwares" : [
    {
      "firmware": "3.61",
      "modules" : [
        {
          "name" : "SceDeci4p",
          "nid" : "0x27",
          "libraries" : [
            {
              "name" : "SceDeci4pUserp",
              "nid" : "0x797E39C0",
              "kernel" : false,
              "functions" : [
                  { "name" : "sceKernelDeci4pClose", "nid" : "0x63B0C50F" },
                  { "name" : "sceKernelDeci4pDisableWatchpoint", "nid" : "0x5A4CDF97" },
                  { "name" : "sceKernelDeci4pEnableWatchpoint", "nid" : "0x3BC66BD8" },
                  { "name" : "sceKernelDeci4pIsProcessAttached", "nid" : "0xC4E1D86D" },
                  { "name" : "sceKernelDeci4pOpen", "nid" : "0x28578FE8" },
                  { "name" : "sceKernelDeci4pRead", "nid" : "0x971E1C66" },
                  { "name" : "sceKernelDeci4pRegisterCallback", "nid" : "0x73371F35" },
                  { "name" : "sceKernelDeci4pWrite", "nid" : "0xCDA3AAAC" },
              ],
              "variables": []
            }
          ]
        },
      ]
    },
  ],
}

@jdek jdek self-assigned this Nov 23, 2016
@frangarcj
Copy link
Contributor

frangarcj commented Nov 24, 2016

FORMAT ULTIMATUM :)

Latest discuted in matrix

{
  "version" : "2",
  "firmware": "3.61",
  "modules" : 
        {
        "SceAudioIn": {
        "nid": "0xA",
        "libraries": {
            "SceAudioIn": {
                "functions": {
                    "sceAudioInGetAdopt": "0x566AC433",
                    "sceAudioInGetStatus": "0x2F940377",
                    "sceAudioInInput": "0x638ADD2D",
                    "sceAudioInOpenPort": "0x39B50DC1",
                    "sceAudioInReleasePort": "0x3A61B8C4"
                },
                "kernel": false,
                "nid": "0xF8DC61A3",
                "variables": {}
            }
        }
    }
  }
}

You have 24 hours to review it :)

@devnoname120
Copy link
Member

devnoname120 commented Nov 24, 2016

I know order is not supposed to matter, but I would be in favor of this:

{
  "layout-version" : "2",
  "firmware": "3.61",
  "modules" : 
        {
        "SceAudioIn": {
        "nid": "0x0000000A",
        "libraries": {
            "SceAudioIn": {
                "nid": "0xF8DC61A3",
                "kernel": false,
                "variables": {},
                "functions": {
                    "sceAudioInGetAdopt": "0x566AC433",
                    "sceAudioInGetStatus": "0x2F940377",
                    "sceAudioInInput": "0x638ADD2D",
                    "sceAudioInOpenPort": "0x39B50DC1",
                    "sceAudioInReleasePort": "0x3A61B8C4"
                }
            }
        }
    }
  }
}

Changes:

  • Moved nid, kernel, and variables.
  • Enforced 0xXXXXXXXX for all NIDs.
  • Renamed version to layout-version to emphasis that this is not the revision of the db itself, but the revision of the layout of the db.

@d3m3vilurr
Copy link
Contributor

can we ignore empty variables field?

@jdek
Copy link
Contributor

jdek commented Nov 25, 2016

These last two formats are ugly af. They're using data as keys.

@frangarcj
Copy link
Contributor

@enoposix we have agreed to use that instead of arrays

@d3m3vilurr yes

@jdek
Copy link
Contributor

jdek commented Nov 25, 2016

@frangarcj why? there's literally no reason not to use a more parseable format, a user isn't going to be using this. And 'we' have not agreed.

@frangarcj
Copy link
Contributor

It is a long irc log but the use of eg. libraries['SceAudioIn'] was proposed for easy searchs. You have a key - value map vs array.

@enoposix What's your opinion or problems?

Sorry for the 'we' thing

@jdek
Copy link
Contributor

jdek commented Nov 25, 2016

@frangarcj I didn't see the benefit of using objects over arrays, but the idea of using it as an object map makes sense, I see where you're coming from now. Acked.

"layout-version" can just be shortened to "version", there shouldn't be any ambiguity as firmware is there too.

@d3m3vilurr
Copy link
Contributor

I saw a opinion about object of array in the IRC, and remembered it only have little benefits.

  1. make more semantic format design
  2. can support unnamed exports

but currently we not support unnamed functions in this project and channel subject changed dynamic loader instead NIDs.

so actually we didn't decided about this.
but I agreed, this issue holded 2 weeks & should make something. it probably better then just holding & no decision.

03:33:49 D<davee[m]> most of the complaints are implementation
03:34:01 D<davee[m]> i find that a good set of work towards a decision talks more
03:34:19 F<frangarcj> I'll make the implementation

format version (or layout-version, and I like just using version) can help this decision.

@xyzz
Copy link
Contributor

xyzz commented Nov 27, 2016

Ok I was away for a week so didn't get the chance to submit my opinion but here it is:

Go with yaml, remove json support and json library from vita-toolchain.

Use hex strings (all caps preferred) instead of decimal numbers

This is dumb and hard to parse.

Easy indeed, we simply need to define it schema, then use it to validate.

I don't see any point of doing any kind of validation. We don't have that many changes to json and if something is broken we'll see it on the build bot anyway.

The format has already been discussed. Most of us prefer YAML for its cleanness, but it's harder to parse because it has complicated features that we don't need (e.g. references, data-type casting, etc.). Since this file is more meant to be read by a program, JSON seemed fine.

We aren't writing our own parser so what's the problem? We aren't even using these advanced features. And we already have a yaml parser for exports written so using it for db.json makes sense.

Parsing yaml in C is a shitfest but most people will use some scripting language, like Python, and there it's just one function call: http://pyyaml.org/wiki/PyYAMLDocumentation


It seems that right now the only person who wants to go with json db w/ hex encoded strings is @devnoname120 and everybody else wants yaml or just does not care, so maybe you can explain his position in more detail?

@yne
Copy link
Contributor

yne commented Nov 27, 2016

Parsing yaml in C is a shitfest

Depend on how strict you want your parser to be.
For example this yaml (converted from psplibdoc.xml long time ago):
https://github.com/yne/prxtool/blob/master/res/psplibdoc.yml
can be parsed using only ~40 line of pure C (who want a yaml dependency anyway):
https://github.com/yne/prxtool/blob/master/db_nids.c#L104

I don't see any point of doing any kind of validation

I'm just giving a solution, I'm not the one who proposed ;-)

@devnoname120
Copy link
Member

There was some discussion on IRC and @xyzz said that YAML is cleaner and we have a working parser for the export.yml files, so we should settle for it.

Here is a proposal for the layout of the imports and exports files. (The DB is an imports file.)
Imports: https://gist.github.com/devnoname120/7f32f510949561a962f0e6b10e88aefd
Exports: https://gist.github.com/devnoname120/da986fa40f9f1c9b0d131c6280684b8b

@frangarcj
Copy link
Contributor

I'm currently converting vita imports to yaml

@frangarcj
Copy link
Contributor

@frangarcj
Copy link
Contributor

frangarcj commented Nov 30, 2016

Changed to yaml. Please check actual yml file

@devnoname120 devnoname120 changed the title Change JSON db format Change NID db format Nov 30, 2016
@devnoname120
Copy link
Member

@frangarcj It seems good to me. There is only the ForDriver --> ForKernel renaming left to do, and maybe the ForKernel --> ksce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants