add Si12T touch driver#1607
Conversation
phoddie
left a comment
There was a problem hiding this comment.
Generally this looks good. It kind of feels like it was translated from C? There are a lot of single use private methods. I marked one place that happens, but there are more. II suggest eliminating those in most cases:
- There is some call overhead
- There is some code overhead
- There is RAM overhead – because each private method uses a RAM slot as JavaScript requires them to be implemented as private fields.
Thanks for the review. You’re right—I did refer to a C implementation, but I also adjusted some parts, like making certain logic more explicit so it works better as comments for Moddable. Still, it looks like there’s more I need to improve, so I’ll take another look. |
|
Thanks for being open to taking another look. I find that small, relatively simple projects like this one are a good opportunity to refine how I organize code.
Interesting. I sensed that the helper function names were kind of serving the purpose of comments. As many people have observed, I'm not a big user of actual comments in code, but they do have zero runtime cost. ;) |
|
I reviewed and removed unnecessary private functions across the entire codebase, not just in the areas that were pointed out. Of course, I’ll also review everything myself, but I’ll incorporate these review learnings into the AI as well. |
|
Thank you for the changes. This is more straightforward to read and has a lighter runtime impact too. Best of both worlds. |
|
This has been merged and will be available in the next code drop. |
This driver is used to pet the M5 StackChan. I don’t plan to add the StackChan device itself, but this driver might be needed elsewhere, so I created a PR.
datasheet