rework DBGLEVEL management

RfidResearchGroup / proxmark3

Iceman Fork - Proxmark3

http://www.icedev.se

GNU General Public License v3.0

3.89k stars 1.03k forks source link

rework DBGLEVEL management #861

Open doegox opened 5 years ago

doegox commented 5 years ago

MF_DBGLEVEL can only be set by hf mf dbg N but it's used across many other technos.

iso14443b
iso15693
iclass
felica
lfops
i2c
fpgaloader
flashmem

We should make the hf mf dbg (and alias hf mfu dbg) a more global command, and make distinction with data setdebugmode

iceman1001 commented 5 years ago

yes, hw debug vs client debug

I started using MF_DBG everywhere on the deviceside since it was a mess before. data debug is also a bad name, It should be client debug, not just related to LF data debugging.

doegox commented 5 years ago

I changed hf mf/mfu dbg to hw dbg. I touched slightly the description of data setdebuglevel. Do we want to move/rename setdebuglevel? If so how/where?

iceman1001 commented 5 years ago

that is a good question. we don't have a client command set, I guess thats why its under data.

For consistency I am all for a rename from setdebuglevel -> clientdbg treated in the way we did the transition of list. Keeping old cmds but they redirect to new place.

What is your take?

doegox commented 5 years ago

I removed entirely hf mf/mfu dbg but we can reintroduce them if you prefer. If we do so, I prefer we print a warning with the new command rather than being a transparent alias, otherwise people will never learn to change their habits.

For setdebuglevel I'm not sure I understood. data setdebuglevel => data clientdbg + keeping data setdebuglevel ?

iceman1001 commented 5 years ago

for mf/mfu dbg , nay, time to let ppl understand its different, keep them removed.

data setdebuglevel -> hw clientdbg ...

iceman1001 commented 5 years ago

how about clientdbg or dbg in root command?

doegox commented 5 years ago

yeah in root that's what I thought of too. dbg is too confusing with hw dbg inho.

pm3 --> clientdbg ?
pm3 --> verbose ?

iceman1001 commented 5 years ago

if Verbose, it could be hooked up to the commands that uses "verbose" or "silent" as parameter :)

iceman1001 commented 5 years ago

or pm3 --> dbg c pm3 --> dbg d

debug client debug device which can be shortend using the shortner to debug c or de h :)

doegox commented 5 years ago

Regarding https://github.com/RfidResearchGroup/proxmark3/commit/2cbe43f269e47cb36817d6b42bb2c76014364e53#commitcomment-34659273 ( all dpbrint* commands should be guarded with DBGLEVEL ifs.) I see two options:

Add DBGLEVEL test in front of each Dbprintf. Tedious and still a risk to forget the next one.
Add a dbglevel parameter to the Dbprintf so it's never forgotten, but this implies the same amount of code changes
Add a DBGLEVEL>NONE in Dbprintf itself, so we're sure just setting DBGLEVEL=NONE will disable all Dbprintf.
another option?

I'm more in favor of the third one...

iceman1001 commented 5 years ago

replace Dbprintf call with a macro, that sets level or not.