Fix kaitai-struct-compiler warnings with rekordbox_pdb.ksy.

[fragmede]

Katai-struct-compiler throws warnings without these changes.

[brunchboy]

Can you provide more context? I get no warnings from the compiler in the build as configured by the project:

--- kaitai:0.1.6:generate (generate) @ crate-digger ---
[INFO] Kaitai: check version
[INFO] kaitai-struct-compiler 0.9
[INFO] Kaitai: generate
[INFO] 
[INFO] --- remotetea:1.1.4:client (jrpcgen.nfs) @ crate-digger ---
...

[fragmede]

Interesting. Using a fresh checkout of kaitai_struct_compiiler, I get

/Users/fragmede/projects/speedier/crate-digger/src/main/kaitai/rekordbox_pdb.ksy: /types/page/instances/num_groups/id:
    warning: use `num_row_groups` instead of `num_groups`, given that it's only used as repeat count of `row_groups` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)

/Users/fragmede/projects/speedier/crate-digger/src/main/kaitai/rekordbox_pdb.ksy: /types/device_sql_long_ascii/seq/2/encoding:
    warning: use canonical encoding name `ASCII` instead of `ascii` (see https://doc.kaitai.io/ksy_style_guide.html#encoding-name)

/Users/fragmede/projects/speedier/crate-digger/src/main/kaitai/rekordbox_pdb.ksy: /types/device_sql_long_utf16le/seq/2/encoding:
    warning: use canonical encoding name `UTF-16LE` instead of `utf-16le` (see https://doc.kaitai.io/ksy_style_guide.html#encoding-name)

/Users/fragmede/projects/speedier/crate-digger/src/main/kaitai/rekordbox_pdb.ksy: /types/device_sql_short_ascii/seq/0/encoding:
    warning: use canonical encoding name `ASCII` instead of `ascii` (see https://doc.kaitai.io/ksy_style_guide.html#encoding-name)

without these changes.

[brunchboy]

Ah, you must be using version 0.10. Unfortunately, I don’t think I can; the Kaitai Maven Plugin uses 0.9, and there has been a PR open against it since 2022 to support 0.10 and later. Hopefully updating the encoding names to be capitalized will be safe with version 0.9, but we would need to test that. As for the change of the num_groups attribute name, can you see if there is a way to disable that warning? It is just opinionated noise, especially since I am not willing to break backwards compatibility with people who are already using the struct.

And thank you for starting this exploration!

[fragmede]

I updated the PR to only capitalize the encoding names. I can live with the warning about num_groups if it causes a ton of downstream changes.

[brunchboy]

Thanks, I saw that. 😸 I am cooking dinner now but will work on finding time to review the documentation for 0.9 and perhaps pull your branch and make sure it doesn’t break beat-link.

[brunchboy]

Looking at the archives of conversations and documentation, it appears that the change to using uppercase encodings is definitely a change. I will need to spend some time this weekend starting a preview train of Crate Digger and making sure that it will not break anything in version 0.9 of the compiler which I am stuck using for the foreseeable future (unless you can get the Maven plugin project to start accepting PRs and working on their issues). In the mean time, if you want to avoid all these warnings, you can use version 0.9 of the compiler yourself.

[fragmede]

That's fine. I didn't realize my tiny fixes were going to result a non-trivial amount of work and just wanted to contribute.

[brunchboy]

No apologies necessary! I very much appreciate your input, and hope to merge your PR! I wish I could use the latest version of the compiler myself, and am annoyed at the Maven plugin project for being stalled, not at you. 😄

[brunchboy]

I have some good news about progress and a curious discovery. By manually specifying the download link in my Maven configuration, I can use version 10 of the compiler with the plugin in my continuous integration. So now I am able to reproduce some of the warnings you reported. Strangely, I am still not seeing the ones about encodings, though, and I am seeing more warnings about disliking the names I have assigned some of my count values. Can you tell me any more about the environment in which you are running the compiler that might account for this difference (I mean specifically about the string encoding names)? What operating system are you using? What version of Java?

As far as renaming the accessors (I’ll quote my full list of warnings below), I am considering doing a backwards-incompatible version bump to adjust those, although it will affect many of my own projects, and anyone else who has been using this mapping, so I will continue to think about that for a bit, and would have to include a big warning in the release notes. I have no way of knowing all the people who are using the mapping, although I am aware of at least a few large projects that are. I really wish there were a way of simply disabling these lint-like warnings, but since Kaitai comes from the Scala community which is highly opinionated, I would not be surprised if there were none.

...
[INFO] KaiTai distribution: Downloading: https://github.com/kaitai-io/kaitai_struct_compiler/releases/download/0.10/kaitai-struct-compiler-0.10.zip
[INFO] KaiTai distribution: Extracting: /Users/jim/.m2/repository/.cache/kaitai/kaitai-struct-compiler-0.10.zip
[INFO] Kaitai: check version
[INFO] kaitai-struct-compiler 0.10
[INFO] Kaitai: generate
[ERROR] /Users/jim/git/crate-digger/src/main/kaitai/rekordbox_pdb.ksy: /types/page/instances/num_groups/id:
[ERROR] warning: use `num_row_groups` instead of `num_groups`, given that it's only used as repeat count of `row_groups` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[ERROR] /Users/jim/git/crate-digger/src/main/kaitai/rekordbox_anlz.ksy: /types/wave_preview_tag/seq/0/id:
[ERROR] warning: use `len_data` instead of `len_preview`, given that it's only used as a byte size of `data` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[ERROR] /Users/jim/git/crate-digger/src/main/kaitai/rekordbox_anlz.ksy: /types/beat_grid_tag/seq/2/id:
[ERROR] warning: use `num_beats` instead of `len_beats`, given that it's only used as repeat count of `beats` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[ERROR] /Users/jim/git/crate-digger/src/main/kaitai/rekordbox_anlz.ksy: /types/cue_extended_tag/seq/1/id:
[ERROR] warning: use `num_cues` instead of `len_cues`, given that it's only used as repeat count of `cues` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[ERROR] /Users/jim/git/crate-digger/src/main/kaitai/rekordbox_anlz.ksy: /types/cue_tag/seq/2/id:
[ERROR] warning: use `num_cues` instead of `len_cues`, given that it's only used as repeat count of `cues` (see https://doc.kaitai.io/ksy_style_guide.html#attr-id)
[INFO] 
[INFO] --- remotetea:1.1.4:client (jrpcgen.nfs) @ crate-digger ---
[INFO] Compiling /Users/jim/git/crate-digger/src/main/rpc/nfs.x
...

[brunchboy]

Ok, I have done more investigation and determined two important things. First, you must be using an unreleased version of the compiler. The changes that produce the warnings about non-canonical encoding names were merged in July of last year, about a year after the last release, which is still version 10. So there is no way for them to affect anyone using releases until the next release.

However, I have been able to verify that switching to the uppercase versions of the encoding names, which will avoid the warnings whenever that release comes out, is backwards compatible with older versions of the compiler, including the one that I am currently using. So I can merge this PR safely to future-proof this mapping file while I continue to think about when and whether to break backwards compatibility by renaming some of my data fields.

Once again, thank you!

[brunchboy]

Oh, whoops, I just realized I need to cancel my GitHub Actions workflow, because it is going to try to build a new version of the library jar with the same version number as an actual release. I will update the project to start preview releases of a new version, and use the improved continuous integration release management code I have built for Beat Link Trigger, this weekend, and then allow a preview jar to build which incorporates this change. That will be a good platform for experimenting with the field name changes as well.

[brunchboy]

And I have started a preview build train for a backwards-incompatible version of Crate Digger, into which I have incorporated the field name changes needed to eliminate all the other linter errors. So if you pull the latest version of the .ksy files you should see no more warnings from ksc.