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

Update decode.py #20

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update decode.py #20

wants to merge 4 commits into from

Conversation

samruddhisk
Copy link

New changes for pulling.

New changes for pulling.
@samruddhisk
Copy link
Author

I have raised a PR with mu code. Please check

Copy link
Owner

@tanmoysrt tanmoysrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the content of PR.
It looks like all the code has been commented, and there is no update in codebase

I have uncommented the code now.
@samruddhisk
Copy link
Author

Please check the content of PR. It looks like all the code has been commented, and there is no update in codebase

I have uncommented the code. please check

@t-anjan
Copy link

t-anjan commented Jul 30, 2024

Just a few comments from my quick review:

  1. Useful comments have been removed.
  2. "Verify email" and "Verify mobile" functions have been removed.
  3. Aadhaar spelling has been wrongly changed, leading to many diffs.

Not sure why all these changes have been done.

@samruddhisk
Copy link
Author

-- I have made changes only to AadhaarSecureQr class. the AadhaarOldQr and AadhaarOfflineXML classes are the same.
--For my requirement as of now verify email and verify mobile is not required, anyways we can add it, not an issue.
--Here priority is to get the data for both old and new Aaadhar QR codes. The same code is working on my local system. You can use my code functionality for reference while making changes to the existing decode.py file in main branch.(As the existing one is working only for old QR, not for new one).
--Aadhaar spelling in AadhaarSecureQr class is 'Aadhaar' as it was before. If there would be any spelling errors I would have got an error while running the code in my local too. However, this code gives me expected results for both old and new OR codes.

@t-anjan
Copy link

t-anjan commented Jul 30, 2024

Can you please provide an example of a new QR which is NOT working with the existing code? And how do you differentiate between old and new QRs?

I have a QR generated in 2023, which is working with the existing code. Hence, I want to understand what is not working exactly.

@samruddhisk
Copy link
Author

Sorry, but I can't share my QR code as it contains my personal information from Aadhaar. I recently downloaded my QR from maadhaar app, that is not working. It says 'string out of index'. My colleague had his QR code downloaded maybe last year, that is working fine.
I would suggest, you download your own new Aadhaar QR code from the app and try with it. As per my observation the field extractions from the new QR data is different than previous one.

@tanmoysrt
Copy link
Owner

-- I have made changes only to AadhaarSecureQr class. the AadhaarOldQr and AadhaarOfflineXML classes are the same. --For my requirement as of now verify email and verify mobile is not required, anyways we can add it, not an issue. --Here priority is to get the data for both old and new Aaadhar QR codes. The same code is working on my local system. You can use my code functionality for reference while making changes to the existing decode.py file in main branch.(As the existing one is working only for old QR, not for new one). --Aadhaar spelling in AadhaarSecureQr class is 'Aadhaar' as it was before. If there would be any spelling errors I would have got an error while running the code in my local too. However, this code gives me expected results for both old and new OR codes.

Please don't remove any functions, only do the changes that you need to modify.

@samruddhisk
Copy link
Author

Sorry for the inconvenience. I have updated the code now.

@samruddhisk
Copy link
Author

I have attached sample outputs for both old and new QR codes which I ran using my code. You can refer those.
You can find a difference in Decompressed array and Delimiters in both the files.

Sample output for new QR.txt
Sample output for old QR.txt

@t-anjan
Copy link

t-anjan commented Aug 2, 2024

I have incorporated some of your changes into this PR: #22 .

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

Successfully merging this pull request may close these issues.

3 participants