Adds sdvxio-bio2 along with bio2drv and aciotest updates
Description
sdvxio-bio2: Implements the sdvxio interface with a BIO2 device flashed with sdvx firmware
bio2drv: Implements the BIO2 specific detection and protocol info (#7)
aciotest: Added BIO2 sdvx polling to test utility
sdvxhook2/iidxhook8: refactor bio2emu
How Has This Been Tested?
Tested on a real sdvx cab with a BIO2, as well as the BIO2 I have on my desk
In GitLab by @xyen on Aug 15, 2020, 04:11
_Merges sdvxio-bio2 -> master_
## Summary
Adds sdvxio-bio2 along with bio2drv and aciotest updates
## Description
sdvxio-bio2: Implements the sdvxio interface with a BIO2 device flashed with sdvx firmware
bio2drv: Implements the BIO2 specific detection and protocol info (#7)
aciotest: Added BIO2 sdvx polling to test utility
sdvxhook2/iidxhook8: refactor bio2emu
## How Has This Been Tested?
Tested on a real sdvx cab with a BIO2, as well as the BIO2 I have on my desk
Nit: Rather odd name considering there is not actual analog input in the struct? Naming suggestions: struct bi2a_sdvx_sys, struct bi2a_sdvx_operator. IIRC I referred to a similar group as inputs as "sys" on IIDX.
Nit: Rather odd name considering there is not actual analog input in the struct? Naming suggestions: `struct bi2a_sdvx_sys`, `struct bi2a_sdvx_operator`. IIRC I referred to a similar group as inputs as "sys" on IIDX.
Good separatation into "bio2", 'bio2emu" and "bio2emu-iidx" 👍
Nit: It took me a moment to figure out that this commit was mainly about this. Pointing that out in the commit message is imo very helpful to understand the context of the changes faster.
Good separatation into "bio2", 'bio2emu" and "bio2emu-iidx" :+1:
Nit: It took me a moment to figure out that this commit was mainly about this. Pointing that out in the commit message is imo very helpful to understand the context of the changes faster.
Since this is the public API of BT, I suggest adding the valid ranges of the parameters here. For now, I think we can stick to what's used internally, 0-96, though that look rather odd, because why not 0-100. shrug
Might be worth a brief comment to explain that we are exposing what the internal API accepts (similar to what we have done on IIDX already).
Since this is the public API of BT, I suggest adding the valid ranges of the parameters here. For now, I think we can stick to what's used internally, 0-96, though that look rather odd, because why not 0-100. *shrug*
Might be worth a brief comment to explain that we are exposing what the internal API accepts (similar to what we have done on IIDX already).
I assume by how it is used, another async thread is polling the IO. If that's the case, I suggest making this atomic to avoid visibility issues when the IO gets shut down.
I assume by how it is used, another async thread is polling the IO. If that's the case, I suggest making this atomic to avoid visibility issues when the IO gets shut down.
only the first grouping is system, the other 4 are analog only.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/bio2/bi2a-sdvx.h line 7](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-1a920aa2e0a8758198f5a03de37c39e7R7)
only the first grouping is system, the other 4 are analog only.
It's unrelated to sdvxio-bio2, it's needed for emulating specific a ICCA versions for games that may need it. iidxhook_util_acio_override_versionwould be called with a particular version number for games that specify it.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/iidxhook-util/acio.h line 25](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-0f0edd006871e553b3b6d7774f444e9dR25)
It's unrelated to sdvxio-bio2, it's needed for emulating specific a ICCA versions for games that may need it. `iidxhook_util_acio_override_version`would be called with a particular version number for games that specify it.
I don't actually know what the real range is, it seems to be 96-0, but I've seen some other code go to 100 as the minimum. Going past 100 is fine too, the device doesn't error out or anything.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/aciodrv/kfca.c line 42](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-8d31ee7b610b97d9222b2c92c080750cL42)
I don't actually know what the real range is, it _seems_ to be 96-0, but I've seen some other code go to 100 as the minimum. Going past 100 is fine too, the device doesn't error out or anything.
It's left there and commented out, so that people don't think that the call is missing. (and so they read the above comment).
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/sdvxhook2/bi2a.c line 112](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-746a3ce80a510b99a97dd57598191af8R112)
It's left there and commented out, so that people don't think that the call is missing. (and so they read the above comment).
I agree, I think I'll mention the range here as well.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/bemanitools/sdvxio.h line 101](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..21d43d6f3dcc4ff370f09fa38b6de3205dba99e5#diff-265296de2ac1356b7bc7c857b8dd8fcdR101)
I agree, I think I'll mention the range here as well.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/sdvxio-bio2/sdvxio.c line 153](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-fa6c2d4b771cf358a54641fe0133b64eR153)
good point
These are based on the KFCA's values (from SDVX1-4) where they are settable individually.
(where the goal of sdvxio-bio2 is to let you play older titles on new cabs).
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [src/main/sdvxio-bio2/sdvxio.c line 322](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-fa6c2d4b771cf358a54641fe0133b64eR322)
These are based on the KFCA's values (from SDVX1-4) where they are settable individually.
(where the goal of sdvxio-bio2 is to let you play older titles on new cabs).
The config file is auto generated, and I think it's pretty clear:
# Autodetect BIO2 port (default: on)
bio2.autodetect=true
# BIO2 serial port
bio2.port=COM4
# BIO2 bus baudrate (real devices expect 115200)
bio2.baud=115200
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [doc/sdvxhook/sdvxio-bio2.md line 16](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-81e1eb1afa80d203d28dd7b422eb3aacR16)
The config file is auto generated, and I think it's pretty clear:
```
# Autodetect BIO2 port (default: on)
bio2.autodetect=true
# BIO2 serial port
bio2.port=COM4
# BIO2 bus baudrate (real devices expect 115200)
bio2.baud=115200
```
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on [doc/sdvxhook/sdvxio-bio2.md line 17](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-81e1eb1afa80d203d28dd7b422eb3aacR17)
see above
Kind of, the goal isn't to prevent the IO getting shut down too early, but to prevent the application from killing itself when there's still a poll in-flight.
The worst case is that the processing_io bit gets read late in sdvx_io_fini which is still fine.
In GitLab by @xyen on Aug 15, 2020, 20:36
Commented on [src/main/sdvxio-bio2/sdvxio.c line 35](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..47e8e7618e59a05ea67cfe14dc0b2f0fc0e6b3a0#diff-fa6c2d4b771cf358a54641fe0133b64eR35)
Kind of, the goal isn't to prevent the IO getting shut down too early, but to prevent the application from killing itself when there's still a poll in-flight.
The worst case is that the processing_io bit gets read late in `sdvx_io_fini` which is still fine.
In GitLab by @xyen on Aug 15, 2020, 20:37
Commented on [src/main/bemanitools/sdvxio.h line 101](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..21d43d6f3dcc4ff370f09fa38b6de3205dba99e5#diff-265296de2ac1356b7bc7c857b8dd8fcdR101)
changed this line in [version 3 of the diff](/djhackers/bemanitools/-/merge_requests/45/diffs?diff_id=1273&start_sha=21d43d6f3dcc4ff370f09fa38b6de3205dba99e5#f3e4768dcdc33b18aad87237ea6765481173e8c2_101_101)
Well, theoretically, the other value might not become visible to the other thread at all, though rather unlikely. I would still like to see the use of an atomic here because we have them since C11, it highlights the awareness of this situation and is simply safe and correct.
Well, theoretically, the other value might not become visible to the other thread at all, though rather unlikely. I would still like to see the use of an atomic here because we have them since C11, it highlights the awareness of this situation and is simply safe and correct.
Only analogs[0] contains the system bits, 1-3 do not.
In GitLab by @xyen on Aug 17, 2020, 04:56
Commented on [src/main/bio2/bi2a-sdvx.h line 7](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..9fa83cef641a1f4e2904b13976f428b419d84261#diff-1a920aa2e0a8758198f5a03de37c39e7R7)
`struct bi2a_sdvx_analog analogs[4];`
Only analogs[0] contains the system bits, 1-3 do not.
In GitLab by @xyen on Aug 17, 2020, 05:59
Commented on [src/main/sdvxio-bio2/sdvxio.c line 35](https://github.com/djhackersdev/bemanitools/compare/3ab55b9ef02a1e052b4ca16c61a021092eced6d1..47e8e7618e59a05ea67cfe14dc0b2f0fc0e6b3a0#diff-fa6c2d4b771cf358a54641fe0133b64eR35)
changed this line in [version 4 of the diff](/djhackers/bemanitools/-/merge_requests/45/diffs?diff_id=1274&start_sha=47e8e7618e59a05ea67cfe14dc0b2f0fc0e6b3a0#3976aee18b933b4c6929999387eb29b49f556486_35_35)
In GitLab by @xyen on Aug 17, 2020, 05:59
added 1 commit
<ul><li>9fa83cef - sdvxio: Add atomics to kfca/bio2</li></ul>
[Compare with previous version](/djhackers/bemanitools/-/merge_requests/45/diffs?diff_id=1274&start_sha=47e8e7618e59a05ea67cfe14dc0b2f0fc0e6b3a0)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
In GitLab by @xyen on Aug 15, 2020, 04:11
Merges sdvxio-bio2 -> master
Summary
Adds sdvxio-bio2 along with bio2drv and aciotest updates
Description
sdvxio-bio2: Implements the sdvxio interface with a BIO2 device flashed with sdvx firmware
bio2drv: Implements the BIO2 specific detection and protocol info (#7)
aciotest: Added BIO2 sdvx polling to test utility
sdvxhook2/iidxhook8: refactor bio2emu
How Has This Been Tested?
Tested on a real sdvx cab with a BIO2, as well as the BIO2 I have on my desk
In GitLab by @xyen on Aug 15, 2020, 04:11
added 8 commits
masterd77deaf7- bio2emu: refactor BIO2 emulationb701bf3c- iidxhook-util: allow setting specified ICCA emulation versiond7c0bd3e- bio2: bio2_bi2a_state -> bi2a_sdvx_stateac908152- sdvx: Allow setting digital amp level from sdvxio837fa049- sdvxio-bio2: Add sdvxio BIO2 along with bio2drv and aciotest updates21d43d6f- doc: add documentation for sdvxio-bio2Compare with previous version
Nit: Rather odd name considering there is not actual analog input in the struct? Naming suggestions:
struct bi2a_sdvx_sys,struct bi2a_sdvx_operator. IIRC I referred to a similar group as inputs as "sys" on IIDX.Good separatation into "bio2", 'bio2emu" and "bio2emu-iidx" 👍
Nit: It took me a moment to figure out that this commit was mainly about this. Pointing that out in the commit message is imo very helpful to understand the context of the changes faster.
Can you briefly explain why this change is required or what it is used for? A brief comment here might be good to have.
Taken from the documentation in the header file, I suggest to have some range checking of the values here and clip them if not within range.
Leftover from testing/debugging?
Since this is the public API of BT, I suggest adding the valid ranges of the parameters here. For now, I think we can stick to what's used internally, 0-96, though that look rather odd, because why not 0-100. shrug
Might be worth a brief comment to explain that we are exposing what the internal API accepts (similar to what we have done on IIDX already).
Good note 👍
To avoid CPU banging, add a sleep(30) (30 ms ?) to the loop.
I assume by how it is used, another async thread is polling the IO. If that's the case, I suggest making this atomic to avoid visibility issues when the IO gets shut down.
I don't get that part. Why are the different volume levels exposed if you can't set them individually?
Can you give an example of how the config entry might look like?
Same here, just an example of some config entries with potential values.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/bio2/bi2a-sdvx.h line 7
only the first grouping is system, the other 4 are analog only.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/iidxhook-util/acio.h line 25
It's unrelated to sdvxio-bio2, it's needed for emulating specific a ICCA versions for games that may need it.
iidxhook_util_acio_override_versionwould be called with a particular version number for games that specify it.In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/aciodrv/kfca.c line 42
I don't actually know what the real range is, it seems to be 96-0, but I've seen some other code go to 100 as the minimum. Going past 100 is fine too, the device doesn't error out or anything.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/sdvxhook2/bi2a.c line 112
It's left there and commented out, so that people don't think that the call is missing. (and so they read the above comment).
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/bemanitools/sdvxio.h line 101
I agree, I think I'll mention the range here as well.
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/sdvxio-bio2/sdvxio.c line 153
good point
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on src/main/sdvxio-bio2/sdvxio.c line 322
These are based on the KFCA's values (from SDVX1-4) where they are settable individually.
(where the goal of sdvxio-bio2 is to let you play older titles on new cabs).
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on doc/sdvxhook/sdvxio-bio2.md line 16
The config file is auto generated, and I think it's pretty clear:
In GitLab by @xyen on Aug 15, 2020, 20:28
Commented on doc/sdvxhook/sdvxio-bio2.md line 17
see above
In GitLab by @xyen on Aug 15, 2020, 20:36
Commented on src/main/sdvxio-bio2/sdvxio.c line 35
Kind of, the goal isn't to prevent the IO getting shut down too early, but to prevent the application from killing itself when there's still a poll in-flight.
The worst case is that the processing_io bit gets read late in
sdvx_io_finiwhich is still fine.In GitLab by @xyen on Aug 15, 2020, 20:37
Commented on src/main/bemanitools/sdvxio.h line 101
changed this line in version 3 of the diff
In GitLab by @xyen on Aug 15, 2020, 20:37
added 9 commits
350ba005- bio2emu: refactor BIO2 emulationccaf053a- iidxhook-util: allow setting specified ICCA emulation version4f43f6db- bio2: bio2_bi2a_state -> bi2a_sdvx_state22c69cf8- Merge branch 'master' of dev.s-ul.eu:djhackers/bemanitoolsb99abc5d- sdvx: Allow setting digital amp level from sdvxioc7817352- sdvxio-bio2: Add sdvxio BIO2 along with bio2drv and aciotest updates600972b8- doc: add documentation for sdvxio-bio2fc290a91- Merge branch 'sdvxio-bio2' of dev.s-ul.eu:djhackers/bemanitools into sdvxio-bio247e8e761- sdvxio-bio2: Add better comments to amp command, and sleep in finiCompare with previous version
what are the other 4 fields? I can only see 3: unk1, unk2 and a_val.
That might be something to add to the comments then.
Well, theoretically, the other value might not become visible to the other thread at all, though rather unlikely. I would still like to see the use of an atomic here because we have them since C11, it highlights the awareness of this situation and is simply safe and correct.
In GitLab by @xyen on Aug 17, 2020, 04:56
Commented on src/main/bio2/bi2a-sdvx.h line 7
struct bi2a_sdvx_analog analogs[4];Only analogs[0] contains the system bits, 1-3 do not.
In GitLab by @xyen on Aug 17, 2020, 05:57
resolved all threads
In GitLab by @xyen on Aug 17, 2020, 05:59
Commented on src/main/sdvxio-bio2/sdvxio.c line 35
changed this line in version 4 of the diff
In GitLab by @xyen on Aug 17, 2020, 05:59
added 1 commit
9fa83cef- sdvxio: Add atomics to kfca/bio2Compare with previous version
approved this merge request
In GitLab by @xyen on Aug 17, 2020, 18:51
merged
In GitLab by @xyen on Nov 11, 2020, 03:07
mentioned in commit a8a43254c5a3eae5d177f56266e4126bd9fc58c9