Separated the key generation and the payload generation core code for rp2 and rp3.
Description
Discussed in the last PR, the rp2 and rp3 code could be merged further. I attempted to separate the code required for the keys from the code that generates the actual dongle payload data.
security-rp2-test and security-rp3-test test cases.
Checklist
Implemented (unit) test(s) which prove that the introduced changes are working as expected.
Tested with the following games:
...
...
Followed the developer (style) guidelines.
Updated existing doc of or add new doc to README file(s).
Updated development documentation.
In GitLab by @33c17f40 on May 12, 2022, 03:45
_Merges rp2_refactor2 -> master_
## Summary
<!--- Provide a general summary of your changes in the Title above -->
Separated the key generation and the payload generation core code for rp2 and rp3.
## Description
Discussed in the last PR, the rp2 and rp3 code could be merged further. I attempted to separate the code required for the keys from the code that generates the actual dongle payload data.
## Related Issue
<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->
https://dev.s-ul.net/djhackers/bemanitools/-/merge_requests/111
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
security-rp2-test and security-rp3-test test cases.
## Checklist
<!-- Make sure you covered all items, which apply, of the checklist below. -->
<!-- Strikethrough items that do not apply and provide a brief description why. -->
* [ ] Implemented (unit) test(s) which prove that the introduced changes are working as expected.
* Tested with the following games:
* [ ] ... <!-- insert game name 1-->
* [ ] ... <!-- insert game name 2--->
* [ ] Followed the developer (style) guidelines.
* [ ] Updated existing doc of or add new doc to README file(s).
* [ ] Updated development documentation.
I feel like you could probably throw security_rp3_generate_signed_eeprom_data into rp2.c but then I'm not sure what to do with the function naming.
Maybe something like this? security_rp2_generate_signed_eeprom_data_v1 security_rp2_generate_signed_eeprom_data_v2
Throwing security_rp3_generate_signed_eeprom_data into rp2.c as-is with that name feels like it breaks the intended file structure because why would you look for rp3 in rp2.c.
Please let me know if you have any ideas.
Edit: Actually, nevermind. Got too focused on the code itself and didn't consider the output. I think separating rp2.c and rp3.c is probably good enough because of the obvious difference in returned data structure.
In GitLab by @33c17f40 on May 12, 2022, 03:50
I feel like you could probably throw `security_rp3_generate_signed_eeprom_data` into rp2.c but then I'm not sure what to do with the function naming.
Maybe something like this?
`security_rp2_generate_signed_eeprom_data_v1`
`security_rp2_generate_signed_eeprom_data_v2`
Throwing `security_rp3_generate_signed_eeprom_data` into rp2.c as-is with that name feels like it breaks the intended file structure because why would you look for rp3 in rp2.c.
Please let me know if you have any ideas.
Edit: Actually, nevermind. Got too focused on the code itself and didn't consider the output. I think separating rp2.c and rp3.c is probably good enough because of the obvious difference in returned data structure.
I guess when you are in doubt right now, the anticipated gain is low and the current structure is not entirely messed up, I suggest to just leave it as is. Further iterations can always be taken when anyone comes back to this.
I guess when you are in doubt right now, the anticipated gain is low and the current structure is not entirely messed up, I suggest to just leave it as is. Further iterations can always be taken when anyone comes back to this.
I'm fine with the PR in its current form. I don't think there's much to gain from further refactoring it.
In GitLab by @33c17f40 on May 12, 2022, 23:28
I'm fine with the PR in its current form. I don't think there's much to gain from further refactoring it.
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 @33c17f40 on May 12, 2022, 03:45
Merges rp2_refactor2 -> master
Summary
Separated the key generation and the payload generation core code for rp2 and rp3.
Description
Discussed in the last PR, the rp2 and rp3 code could be merged further. I attempted to separate the code required for the keys from the code that generates the actual dongle payload data.
Related Issue
https://dev.s-ul.net/djhackers/bemanitools/-/merge_requests/111
How Has This Been Tested?
security-rp2-test and security-rp3-test test cases.
Checklist
In GitLab by @33c17f40 on May 12, 2022, 03:50
I feel like you could probably throw
security_rp3_generate_signed_eeprom_datainto rp2.c but then I'm not sure what to do with the function naming.Maybe something like this?
security_rp2_generate_signed_eeprom_data_v1security_rp2_generate_signed_eeprom_data_v2Throwing
security_rp3_generate_signed_eeprom_datainto rp2.c as-is with that name feels like it breaks the intended file structure because why would you look for rp3 in rp2.c.Please let me know if you have any ideas.
Edit: Actually, nevermind. Got too focused on the code itself and didn't consider the output. I think separating rp2.c and rp3.c is probably good enough because of the obvious difference in returned data structure.
I guess when you are in doubt right now, the anticipated gain is low and the current structure is not entirely messed up, I suggest to just leave it as is. Further iterations can always be taken when anyone comes back to this.
lgtm, waiting until the thread is resolved by you to give you the chance to reply if there is anything else to bring up.
approved this merge request
In GitLab by @33c17f40 on May 12, 2022, 23:28
I'm fine with the PR in its current form. I don't think there's much to gain from further refactoring it.
resolved all threads