-
Notifications
You must be signed in to change notification settings - Fork 30
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
Sync: commaai/panda:master
into sunnypilot/panda:master-new
#61
Conversation
Reviewer's Guide by SourceryThis pull request synchronizes the Class diagram for updated CAN and UDS communication classesclassDiagram
class CanClient {
+tx_addr: int
+rx_addr: int
+rx_buff: deque[bytes]
+sub_addr: int
+bus: int
+debug: bool
+__init__(can_send, can_recv, tx_addr, rx_addr, bus, sub_addr, debug)
+send(msgs: list[bytes], delay: float)
}
class IsoTpMessage {
-_can_client: CanClient
-_timeout: float
-_debug: bool
-_max_len: int
+__init__(can_client, timeout, single_frame_mode, separation_time, debug, max_len)
}
class UdsClient {
+bus: int
+__init__(panda, tx_addr, rx_addr, bus, sub_addr, timeout, debug, tx_timeout, response_pending_timeout)
}
IsoTpMessage --> CanClient
UdsClient --> CanClient
note for CanClient "Updated rx_buff type annotation"
note for IsoTpMessage "Removed parentheses from class definition"
note for UdsClient "Removed parentheses from class definition"
State diagram for Cuatro board boot statesstateDiagram-v2
[*] --> BOOT_BOOTKICK: GPIO A0 Active Low
[*] --> BOOT_RESET: GPIO C12 Active Low
note right of BOOT_RESET: Now enabled in bootkick function
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @sunnyhaibin - I've reviewed your changes - here's some feedback:
Overall Comments:
- The bootkick reset code in cuatro.h was previously commented out with 'only use if we have to'. Please validate and confirm this functionality is needed before enabling it.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Enhancements:
CanClient
andIsoTpMessage
classes.