-
Notifications
You must be signed in to change notification settings - Fork 8
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
✨ (drivers): Add CoreDACExpander #828
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #828 +/- ##
===========================================
+ Coverage 95.50% 95.59% +0.08%
===========================================
Files 116 118 +2
Lines 2649 2703 +54
===========================================
+ Hits 2530 2584 +54
Misses 119 119
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff listNo differenes where found in map files. |
File comparision analysis report🔖 Info
Click to show memory sections
📝 SummaryClick to show summary
🗺️ Map files diff outputClick to show diff listNo differenes where found in map files. |
d68ff5f
to
ad635a2
Compare
ad635a2
to
7fa9668
Compare
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.
Rediscuter ensemble de la segmentation entre AnalogOut et calibration toucher + les conséquences que ça va avoir dans l'utilisation du composant (utilisation partielle)
En tout cas, beau travail pour avoir bien balayer les fonctionnalités du composant 👏
|
||
namespace leka::interface { | ||
|
||
class DACTouch |
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.
Il y a 2 choses qui se mélange ici et je pense qu'il serait important de les séparer:
- La partie DAC ou nommé AnalogOut sous mbed
- La calibration du capteur de toucher, ici grâce à une entrée analogique
A cela, tu as ajouté le fonctionnement du driver utilisé aujourd'hui:
- voltage reference
- powermode
- gain
- ...
Or un driver, que ce soit le DAC ou la calibration du capteur de toucher, ne devrait pas présenter des fonctions spécifique à un matériel mais se doivent être générique et c'est l'implémentation qui va adapter le matériel pour qu'il soit conforme au driver, et ici à l'interface.
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.
je rejoins @YannLocatelli !
il faut bien séparer les concepts:
- le
io_expander::AnalogOut
- le calibrateur qui utilise le AnalogOut
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.
Premier passage
|
||
namespace command { | ||
|
||
inline constexpr uint8_t writeAllInputRegister = 0x00; |
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.
même chose que dans #792 pour ici et pour tous les autres:
inline constexpr uint8_t writeAllInputRegister = 0x00; | |
inline constexpr auto writeAllInputRegister = uint8_t {0x00}; |
void CoreDACTouchMCP4728::writeToAllInputRegister(std::array<uint8_t, 2> value) | ||
{ | ||
auto command = std::array<uint8_t, 2> {}; | ||
command.at(0) = static_cast<uint8_t>((0xC0 & mcp4728::command::writeAllInputRegister) | (0x3F & value.at(0))); |
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.
les &
et |
je comprends pas ce que ça fait.
est-ce que tu peux avoir une petite fonction privée avec un nom explicite de l'opération.
{ | ||
auto command = std::array<uint8_t, 3> {}; | ||
command.at(0) = | ||
static_cast<uint8_t>((0xF8 & mcp4728::command::writeSpecificInputRegister) | (0x07 & ((0x03 & channel) << 1))); |
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.
pareil ici, pourquoi 0xF8 &
et pas autre chose?
pourquoi (0x07 & ((0x03 & channel) << 1)))
et pas autre chose.
faut que le nom de l'opération soit explicite
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.
et pareil pour les autres en dessous qui sont parfois les mêmes.
|
||
namespace leka::interface { | ||
|
||
class DACTouch |
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.
je rejoins @YannLocatelli !
il faut bien séparer les concepts:
- le
io_expander::AnalogOut
- le calibrateur qui utilise le AnalogOut
00778d5
to
ccf94ac
Compare
ccf94ac
to
05b6441
Compare
Kudos, SonarCloud Quality Gate passed! |
A supprimer/fermer : se référer à |
No description provided.