Conversation
nedseb
left a comment
nedseb
left a comment
There was a problem hiding this comment.
Bonne base, le README couvre les fonctionnalités principales. Cependant plusieurs erreurs factuelles à corriger :
Erreurs dans les noms de fichiers exemples
La table des exemples contient 4 noms de fichiers incorrects sur 15. Compare avec le contenu réel de examples/ :
| Dans le README (faux) | Fichier réel |
|---|---|
framebuf_pixel.py |
framebuf_pixels.py |
framebuf_rect.py |
framebuf_rects.py |
random_pixel.py |
random_pixels.py |
rotating_3D_cube.py |
rotating_3d_cube.py |
Rappel important : avant de soumettre, lance un simple ls lib/ssd1327/examples/ et vérifie que chaque nom correspond. Ce type d'erreur est typique d'un copier-coller depuis un LLM sans vérification.
Descriptions d'exemples trop vagues
"Draw a line", "Draw a pixel", "Display a text" — ces descriptions répètent le nom du fichier sans rien ajouter. Propose des descriptions qui expliquent ce que l'exemple montre concrètement (ex: "Draw lines with different greyscale levels across the screen").
Méthodes manquantes dans l'API Reference
lookup(data)— méthode publique pour modifier la table de niveaux de gris, non documentéereset()— méthode surSSD1327_SPIpour reset hardware via la pin, non documentée
Méthodes internes exposées
write_cmd() / write_data() sont listées dans l'API publique avec la mention "(Implemented internally)". Si ce sont des méthodes internes, retire-les de la section API Reference. Si elles sont utiles au développeur, documente-les correctement avec signature et paramètres.
Cohérence avec les autres README
- Section I²C Address : un bloc de code avec juste
0x3Cest disproportionné. Une ligne "Default I²C address:0x3C" ou un retour dans le tableau des specs suffit. - Section Notes : les blocs de code pour la formule de taille du buffer et pour
show()sont excessifs. Une phrase inline suffit : "The internal buffer useswidth × height / 2bytes (4-bit per pixel). Callshow()to push the buffer to the display." - Pas de section "Supported Display" : les autres README harmonisés ont une section dédiée au composant avec référence exacte et caractéristiques.
Source
"This library is a port of the original MicroPython SSD1327 driver." — la référence est tout en bas du fichier. Soit mets le lien directement dans cette phrase d'intro (ex: "This library is a port of micropython-ssd1327."), soit supprime cette phrase d'intro vague.
There was a problem hiding this comment.
Pull request overview
Expands the ssd1327 driver README to provide feature/specification details, usage guidance, and a more complete API/examples reference, aligning it more closely with other driver documentation in the repo.
Changes:
- Added Features, Display Specifications, I2C Address, Basic Usage, FrameBuffer Integration, API Reference, and Notes sections.
- Added a more comprehensive Examples table and an mpremote invocation snippet.
- Updated source attribution section and general formatting/consistency.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * rotation | ||
| * Power management: | ||
| * display on/off | ||
| * Optimized buffer rendering |
There was a problem hiding this comment.
The feature bullet "Optimized buffer rendering" looks inaccurate for this driver: show() always writes the full framebuffer (self.write_data(self.buffer)), with no partial/dirty-rectangle updates. Consider rewording this bullet to avoid implying incremental rendering, or document what is actually optimized here (if anything).
| * Optimized buffer rendering | |
| * Framebuffer-based rendering API |
| The driver internally uses `framebuf.FrameBuffer`, meaning **all standard drawing methods are available**. | ||
|
|
||
| This allows you to use: | ||
|
|
||
| * `fill()` | ||
| * `pixel()` | ||
| * `line()` | ||
| * `rect()` | ||
| * `text()` | ||
| * `scroll()` |
There was a problem hiding this comment.
The FrameBuffer integration section claims "all standard drawing methods are available" and lists rect(), but the driver only exposes a small wrapper set (fill, pixel, line, scroll, text). Other FrameBuffer methods (e.g., rect, fill_rect) are only available via display.framebuf.* in the current implementation. Please update the documentation to match the actual API (or add forwarding methods).
| | framebuf_lines.py | Draw a line | | ||
| | framebuf_pixel.py | Draw a pixel | | ||
| | framebuf_rect.py | Draw a rectangle | | ||
| | framebuf_scroll.py | Scroll a text | | ||
| | framebuf_text.py | Display a text | | ||
| | hello_world.py | Display "Hello World" | | ||
| | illusion.py | Display an optic illusion | | ||
| | invert.py | invert greyscale lookup table | | ||
| | lookup_table.py | Display gradiant | | ||
| | micropython_logo.py | Display micropython logo | | ||
| | random_pixel.py | Display random pixels | | ||
| | rotating_3D_cube.py | Display a rotating 3D cube | | ||
| | rotation.py | Rotate the 3D logo | |
There was a problem hiding this comment.
Several example filenames in the table don't match the actual files in lib/ssd1327/examples/ (e.g., framebuf_pixels.py, framebuf_rects.py, random_pixels.py, rotating_3d_cube.py). Please update the table so each row links to/mentions the correct filename, otherwise users will get "file not found" when trying to run them.
| | hello_world.py | Display "Hello World" | | ||
| | illusion.py | Display an optic illusion | | ||
| | invert.py | invert greyscale lookup table | | ||
| | lookup_table.py | Display gradiant | |
There was a problem hiding this comment.
Typo in the examples table description: "gradiant" should be "gradient".
| | lookup_table.py | Display gradiant | | |
| | lookup_table.py | Display gradient | |
| ```python | ||
| SSD1327_SPI(width, height, spi, dc, res, cs) | ||
| WS_OLED_128X128_SPI(spi, dc, res, cs) | ||
| ``` |
There was a problem hiding this comment.
The README documents SSD1327_SPI(...), but SSD1327_SPI is not exported by ssd1327/__init__.py (only WS_OLED_128X128_SPI is). As written, from ssd1327 import SSD1327_SPI / ssd1327.SSD1327_SPI won’t work. Either export SSD1327_SPI as part of the public API or adjust the docs to reference the correct import path / supported public classes.
|
J'ai réécrit le README pour traiter tous les commentaires (revue + Copilot) en 6ac947c : Corrections de la revue :
Corrections Copilot :
La PR est prête à merger. |
|
🎉 This PR is included in version 0.0.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Closes #192
Parent issue #194
Description
Expand SSD1327 README to match other drivers documentation level.
Changes
Checklist