RfidResearchGroup / ChameleonUltra

The new generation chameleon based on NRF52840 makes the performance of card emulation more stable. And gave the chameleon the ability to read, write, and decrypt cards.
https://chameleonultra.com
GNU General Public License v3.0
922 stars 153 forks source link

[Bug] Slot Nick name starts at 1 not 0 #51

Closed F9Alejandro closed 1 year ago

F9Alejandro commented 1 year ago

Issue:

using command hw slot nick get -s 1 -st 1 starts at 1 instead of expected 0.

Expected:

Returns slot 1 nickname

Actual:

Returns slot 2 nickname (if it exists)

This happens with all slots. When checking the gui apps it shows slot 2 and above have been named instead of the intended slot 1. Changing the sent slot value from slot_num to int(slot_num-1) works as expected. Not an ideal solution though. if this could be done firmware side instead this would make it easier and will prevent the need to modify the cli code and add hardcoded values.

Before change: CleanShot 2023-08-10 at 21 41 23@2x

CleanShot 2023-08-10 at 21 42 32@2x

After change: CleanShot 2023-08-10 at 21 43 56@2x

F9Alejandro commented 1 year ago

This needs further investigation. There could be a bug in the way GUI apps grab information from the CU and is causing issues with how data is displayed. (Nick slots could start from 0 instead of the intended 1 based on how the cli works)

xianglin1998 commented 1 year ago

The chameleon slot offset is from 0 start(in firmware). However, when using a chameleon in practice, it is necessary to map the card slot to the card slot number printed on the hardware.

xianglin1998 commented 1 year ago

So, this is py CLI bug.

F9Alejandro commented 1 year ago

Thank you for the heads up. I know a quick and dirty way to fix it but I believe it would be better to put it as a const enum instead that remaps int 1 to int 0 for nick specific.

xianglin1998 commented 1 year ago

Slot index decrement image

But slot function lost it.

image

F9Alejandro commented 1 year ago

That makes more sense! Thank you for the clarification.

xianglin1998 commented 1 year ago

Yes, enum type is a strong solution.

F9Alejandro commented 1 year ago

Best to use enums for the slot in general if that is the case. It makes it easier to change if the firmware were to make a change to how information is being stored.

xianglin1998 commented 1 year ago

If I have time, I will refactor it using enumeration types, or if you know how to write PY script, you can push a PR, and the community will be very grateful to you.

F9Alejandro commented 1 year ago

I will have to learn how python does their enums. Mainly been using TS, C++, Rust, and Golang. It is possible to make the change. I could also work with szymex73 on getting this working as well. They are currently working on an overhaul to the way how the cli functions with autocomplete and fixes the issue with using any other keys except for left and right arrows making the line messed up.

xianglin1998 commented 1 year ago

image

F9Alejandro commented 1 year ago

Please see #52 as this should be able to fix the issue that is happening. I didn't do extensive testing mainly just the 'hw slot nick get' command with '-h' and '-s -st' just to make sure it doesn't crash or do something it shouldn't. Since it will only accept a number 1 - 8 there should be no issue with the conversion from name to value (function should really be called ntov instead of fix now that I think about it)

xianglin1998 commented 1 year ago

Perfect, i need some time to test all cmd for slot index required. LoL

F9Alejandro commented 1 year ago

Closing this as the PR has been merged and I got some valuable insight from @doegox. Thank you again!