CaringCaribou / caringcaribou

A friendly car security exploration tool for the CAN bus
GNU General Public License v3.0
751 stars 197 forks source link

Add decoded NRC text to negative response messages #115

Closed alexdetrano closed 3 months ago

alexdetrano commented 4 months ago

See https://github.com/CaringCaribou/caringcaribou/pull/114/commits/8cf8f6528cd60262db5707927cd17809044d0604

The gist of it is that anytime the ECU responds with a negative response code (NRC), we should inform the user what that code is and what the human readable version of that code is. For example an NRC of 0x33 means "Security Access Denied". Currently there is some code only for the ReadMemoryByAddress function, but I'm filing this issue as an enhancement so that all UDS functions will respond with similar messages.

the following changes should be made:

kasperkarlsson commented 4 months ago

Hi there - yesterday I missed that you created a separate issue for this enhancement suggestion (great that you did this by the way - my bad that I didn't notice until now), so I responded in https://github.com/CaringCaribou/caringcaribou/pull/114#issuecomment-2229738119 🙂

There is already a lookup dict, which can be used as NRC_NAMES.get(nrc) (corresponding to your suggested nrc_to_text(nrc)). Implementing an additional function which also prints a message containing the NRC name sounds like a good idea, I'll try it out and reach back!

kasperkarlsson commented 4 months ago

I implemented a wrapper function get_negative_response_code_name(nrc) as you suggested, which returns the NRC name for a given NRC value. Your other suggestion regarding the print function was implemented as print_negative_response_code(nrc). I also made sure these are used consistently throughout the module, rather than NRC lookup being spread out across several similar implementations and having to specify default values explicitly everywhere 😅

Feel free to have a look at the branch https://github.com/CaringCaribou/caringcaribou/tree/uds-nrc-print-function (diff here https://github.com/CaringCaribou/caringcaribou/compare/master...uds-nrc-print-function ) and let me know what you think! Once we are satisfied I'll make a PR and merge it into master.

kasperkarlsson commented 3 months ago

Implemented in #119 👍