Discussion :

[electroremy]

Bonjour,

je suis en train de modifier le firmware de mon imprimante 3D Prusa I3 Original MK2s

il a été compilé avec l'IDE Arduino 1.6

Je suis arrivé à trouver et à désactiver deux fonctions, dans le but de pouvoir installer une sonde SUPERPINDA à la place de la PINDA V1

Je souhaite en profiter pour améliorer un point que je trouve très agaçant de l'interface : le réglage des températures

Qu'il s'agisse de la température du lit ou de la buse, il faut régler une valeur élevée (100°C pour le bed et 200°C à 260°C pour la buse) à partir de zéro, en tournant un bouton cranté dont chaque cran augmente la température de 1°C

Le code concerné utilise des macros que je ne comprends pas bien.

Voici les lignes de codes qui sont appelées pour régler la température avec le fameux bouton :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
  MENU_ITEM_EDIT(int3, MSG_NOZZLE, &target_temperature[0], 0, HEATER_0_MAXTEMP - 10);//3 
  MENU_ITEM_EDIT(int3, MSG_BED, &target_temperature_bed, 0, BED_MAXTEMP - 10);//4

Voici les macros que l'on trouve plus haut :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
/* Helper macros for menus */
#define START_MENU() do { \
    if (encoderPosition > 0x8000) encoderPosition = 0; \
    if (encoderPosition / ENCODER_STEPS_PER_MENU_ITEM < currentMenuViewOffset) currentMenuViewOffset = encoderPosition / ENCODER_STEPS_PER_MENU_ITEM;\
    uint8_t _lineNr = currentMenuViewOffset, _menuItemNr; \
    bool wasClicked = LCD_CLICKED;\
    for(uint8_t _drawLineNr = 0; _drawLineNr < LCD_HEIGHT; _drawLineNr++, _lineNr++) { \
      _menuItemNr = 0;
 
#define MENU_ITEM(type, label, args...) do { \
    if (_menuItemNr == _lineNr) { \
      if (lcdDrawUpdate) { \
        const char* _label_pstr = (label); \
        if ((encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) == _menuItemNr) { \
          lcd_implementation_drawmenu_ ## type ## _selected (_drawLineNr, _label_pstr , ## args ); \
        }else{\
          lcd_implementation_drawmenu_ ## type (_drawLineNr, _label_pstr , ## args ); \
        }\
      }\
      if (wasClicked && (encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) == _menuItemNr) {\
        lcd_quick_feedback(); \
        menu_action_ ## type ( args ); \
        return;\
      }\
    }\
    _menuItemNr++;\
  } while(0)
 
#define MENU_ITEM_DUMMY() do { _menuItemNr++; } while(0)
#define MENU_ITEM_EDIT(type, label, args...) MENU_ITEM(setting_edit_ ## type, label, (label) , ## args )
#define MENU_ITEM_EDIT_CALLBACK(type, label, args...) MENU_ITEM(setting_edit_callback_ ## type, label, (label) , ## args )
#define END_MENU() \
  if (encoderPosition / ENCODER_STEPS_PER_MENU_ITEM >= _menuItemNr) encoderPosition = _menuItemNr * ENCODER_STEPS_PER_MENU_ITEM - 1; \
  if ((uint8_t)(encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) >= currentMenuViewOffset + LCD_HEIGHT) { currentMenuViewOffset = (encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) - LCD_HEIGHT + 1; lcdDrawUpdate = 1; _lineNr = currentMenuViewOffset - 1; _drawLineNr = -1; } \
  } } while(0)

Ce que je comprends, c'est que MENU_ITEM_EDIT est une macro qui permet de régler une valeur avec le bouton, avec une valeur maxi et une valeur mini.

Pour moi ce n'est pas très clair, je ne maitrise pas bien les macros en C++
Si j'avais écris ce code, j'aurais utilisé des fonctions à la place de ces macros.

Je voudrais changer le comportement pour le réglage de la température du lit (bed) et de la buze (nozzle) pour avoir des fonctions séparées, mais je ne sais pas comment réécrire ces fonctions séparément (donc sans macro) pour ensuite les modifier

Une fois ce travail de réécriture faite, ce sera facile de faire ce que je veux, à savoir :

Pour la température de la buse : quand elle est à zéro (démarrage de l'imprimante ou arrêt de l'impression), que dès le premier clic de rotation du bouton dans le sens des aguilles d'une montre je passe directement à 180°C - et inversement, si je tourne le bouton dans le sens anti-horaire, que ça passe à zéro juste après 180°

Pour la température du lit : avoir des pas de 5°C jusque 95°C et ensuite avoir des pas de 1°C

Je pense également changer l'agencement des menus et sous-menus, les fonctions que j'utilise souvent sont dans des menus différents c'est casse-pied.
Il s'agit aussi de préserver la durée de vie du fameux bouton rotatif.

Merci - à bientôt

[f-leb]

Bonjour,

Bigre... ça c'est de la macro

J'ai testé avec un compilateur en ligne (godbolt.org) avec l'option de compilation -E pour voir le résultat des macros transformées en C.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

MENU_ITEM_EDIT(int3, MSG_NOZZLE, &target_temperature[0], 0, HEATER_0_MAXTEMP - 10);//3

devient (après formatage) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
  do {
    if (_menuItemNr == _lineNr) {
      if (lcdDrawUpdate) {
        const char * _label_pstr = (MSG_NOZZLE);
        if ((encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) == _menuItemNr) {
          lcd_implementation_drawmenu_setting_edit_int3_selected(_drawLineNr, _label_pstr, (MSG_NOZZLE), & target_temperature[0], 0, HEATER_0_MAXTEMP - 10);
        } else {
          lcd_implementation_drawmenu_setting_edit_int3(_drawLineNr, _label_pstr, (MSG_NOZZLE), & target_temperature[0], 0, HEATER_0_MAXTEMP - 10);
        }
      }
      if (wasClicked && (encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) == _menuItemNr) {
        lcd_quick_feedback();
        menu_action_setting_edit_int3((MSG_NOZZLE), & target_temperature[0], 0, HEATER_0_MAXTEMP - 10);
        return;
      }
    }
    _menuItemNr++;
  } while (0);

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

MENU_ITEM_EDIT(int3, MSG_BED, &target_temperature_bed, 0, BED_MAXTEMP - 10);//4

devient :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
  do {
    if (_menuItemNr == _lineNr) {
      if (lcdDrawUpdate) {
        const char * _label_pstr = (MSG_BED);
        if ((encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) == _menuItemNr) {
          lcd_implementation_drawmenu_setting_edit_int3_selected(_drawLineNr, _label_pstr, (MSG_BED), & target_temperature_bed, 0, BED_MAXTEMP - 10);
        } else {
          lcd_implementation_drawmenu_setting_edit_int3(_drawLineNr, _label_pstr, (MSG_BED), & target_temperature_bed, 0, BED_MAXTEMP - 10);
        }
      }
      if (wasClicked && (encoderPosition / ENCODER_STEPS_PER_MENU_ITEM) == _menuItemNr) {
        lcd_quick_feedback();
        menu_action_setting_edit_int3((MSG_BED), & target_temperature_bed, 0, BED_MAXTEMP - 10);
        return;
      }
    }
    _menuItemNr++;
  } while (0);

voilà, facile

[Guesset]

Bonjour,

Je peine à comprendre l'intérêt de telles macros. Certes, cela évite le coût d'appel de fonctions.

Mais sil il n'y a qu'un seul appel, pourquoi ne pas inclure le code C équivalent en lieu et place ?

Et s'il y a plusieurs appels, la macro provoquera une duplication de code. Je pense qu'une fonction déclarée inline serait plus appropriée et plus lisible même si le compilateur peut choisir (à bon escient ?) d'ignorer la demande.

Très joli le do ... while(0) . Par conséquence, le _menuItemNr++; est dans une boucle qui ne tourne pas. Peut-être opérationnel selon l'endroit où le code sera expansé mais difficile à savoir.

Salutations

[electroremy]

Merci beaucoup pour vos réponses

Oui, comme avec les macros le code est dupliqué, je ne vois pas trop l'intérêt, c'est même trompeur : cela donne l'impression de faire du code optimisé plus compact alors que les macros demandent au post-processeur de faire des copiers coller.

Je trouve que l'usage à outrance des macro nuit beaucoup à la lisibilité du code.
Surtout que beaucoup de blocs #ifdef... ne sont pas indentés

Et encore, j'ai du bol, ce firmware est ancien et n'a été écrit que pour un modèle d'imprimante 3D avec quelques variantes.

La version suivante est un code versatile qui peut être compilé pour de nombreux modèles mais le code est bourré de #ifdef, avec comme autre conséquence des fichiers sources avec de nombreuses lignes, beaucoup de fonctions s'étalent sur plusieurs écrans. A chaque étape de la fonction, on doit se farcir la logique des #ifdef, ce qui rend bien plus difficile de voir ce que fait la fonction.

Quand on fait du reverse engineering c'est l'horreur - car même si ce code est open source, il n'y pas de vraiement de documentation à jour sur cette version pour vraiement expliquer la structure du code et le pourquoi du comment.


Les macros c'est bien pour définir des constantes ou certains cas particuliers (du style PROGMEM) mais je trouve que ce serait plus lisible et plus efficace de n'utiliser que des fonctions, et pour gérer plusieurs versions de matériel, d'utiliser des fichiers séparés, chacun contenant une variante des fonctions adatpées au matériel, en les sélectionnant avec juste des include qui vont bien.

L'autre aspect est que ce projet Open Source a de nombreux contributeurs, beaucoup d'entres eux n'ont pas une compréhension globale et ont travaillé sur un problème précis. Conséquence : il y a la trouille - légitime - de ne pas toucher à ce qu'on ne comprends pas, en insérant un bout de code dans un projet existant on s'adapte et on réutilise les fonctions du code existant, en laissant, par sécurité, des paramètres qui peuvent ne pas être utilie, sans que ce soit forcément expliqué dans les commentaires. Difficile pour une autre personne qui va relire ce code de comprendre, qu'en fait, les paramètres A, B et C n'ont en fait aucun rôle mais ils sont là pour des raison de compatubilité avec le reste du code, qui lui utilise ces paramètres.


Après, en utilisant NotePad++ je n'ai peut-être pas un bon outil d'édition pour traiter de tels fichiers.

A voir si un outil fiable permet de "purger" les fichiers sources en éliminant certains blocs de codes entre #ifdef (mais pas tous, il faudrait pouvoir choisir) et bien sûr en préservant la mise en page et les commentaires.

[Guesset]

Bonjour,

Ce code ancien est accessible où ? Merci.

Salutations

[electroremy]

Bonjour,

Ce code ancien est accessible où ? Merci.

Salutations

Il est ici :

https://github.com/prusa3d/Prusa-Firmware/tree/v3.2.3

Le schéma ici : https://github.com/ultimachine/Mini-.../board/Project Outputs for Mini-Rambo/Mini-Rambo.PDF

[Obsidian]

Hello à tous,

Je peine à comprendre l'intérêt de telles macros. Certes, cela évite le coût d'appel de fonctions. Mais sil il n'y a qu'un seul appel, pourquoi ne pas inclure le code C équivalent en lieu et place ? Et s'il y a plusieurs appels, la macro provoquera une duplication de code. Je pense qu'une fonction déclarée inline serait plus appropriée et plus lisible même si le compilateur peut choisir (à bon escient ?) d'ignorer la demande.

inline n'a pas toujours existé en C pur. Le mot-clé a été rétroporté du C++ et introduit avec C99 (tout comme les commentaires //), mais aujourd'hui, on peut considérer que c'est acquis de longue date.

Il est parfois nécessaire de recourir aux macros pour envelopper les parties qui ne peuvent pas être directement intégrées aux fonctions mais en général, on se débrouille pour les réduire à la portion congrue. Les seuls avantages que je vois ici à déclarer des macros aussi longues sont :

1. De ne pas avoir de dépendance à une bibliothèque tierce, statique ou dynamique, qui nécessiterait une édition des liens dans tous les cas (surtout si ça ne concerne que quelques macros) ;
2. Pouvoir au besoin rebasculer de façon transparente vers des appels classiques lorsque ces fonctionnalités sont présentes dans l'environnement de travail.

Si l'on voulait s'en tenir à des fonctions classiques sans recourir au linker, alors il faudrait inclure la définition (donc le corps) des fonctions concernées dans un *.h et ce n'est généralement pas souhaitable.

Très joli le do ... while(0) . Par conséquence, le _menuItemNr++; est dans une boucle qui ne tourne pas.

Attention, c'est un do…while et pas un while seul. La boucle est donc toujours exécutée une et une seule fois. C'est une forme très fréquemment utilisée dans les macros de ce genre pour respecter la compatibilité au niveau syntaxique. Sinon, on pourrait se retrouver avec des incohérences dans le code difficiles à débuguer une fois la macro développée.

Ça permet par exemple d'imposer l'ajout d'un point-virgule à la fin de la macro comme s'il s'agissait d'un appel de fonction ordinaire, ce qui est pratique lorsque l'on veut substituer l'un à l'autre en fonction du contexte. Ça permet également à la macro d'ouvrir proprement un bloc si elle contient plusieurs instructions. Sinon, dans le cas d'un if (condition) APPEL_MACRO();, seule la première instruction de la macro serait soumise à la condition et les autres s'exécuteraient dans tous les cas.

[Guesset]

Bonjour Obsidian,

...Attention, c'est un do…while et pas un while seul. La boucle est donc toujours exécutée une et une seule fois. C'est une forme très fréquemment utilisée dans les macros de ce genre pour respecter la compatibilité au niveau syntaxique. Sinon, on pourrait se retrouver avec des incohérences dans le code difficiles à débuguer une fois la macro développée.

Bien sûr qu'il y a un passage, mais de simples accolades, du reste présentes, suffisent pour définir le bloc. Il y aurait peut-être un message d'information du compilateur si la macro est elle-même appelée entre accolades, mais sans réelle importance.

Par ailleurs, le return n'est pas très sûr : la macro peut sortir de la fonction où elle est "appelée" sans que ce soit visible directement dans le code de cette fonction (en espérant que ce n'est pas le main ). Utiliser des accolades pour d'éventuelles incohérences, mais laisser un codage de ce type laisse perplexe.

Salut

[electroremy]

Une remarque :

C'est du C++ à la sauce Arduino, qui sera compilé pour un microcontroleur 8 bits

Parfois, pour optimiser la taille et la vitesse du code, on est amené à coder de manière un peu étrange

[electroremy]

Bonjour,

Bigre... ça c'est de la macro

J'ai testé avec un compilateur en ligne (godbolt.org) avec l'option de compilation -E pour voir le résultat des macros transformées en C.

J'ai remplacé les macros par les fonctions transformées en C

La compilation fonctionne, les binaires font exactement la même taille

Le "jeu de piste" continue

Le code utilise les trois fonctions suivantes :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
lcd_implementation_drawmenu_setting_edit_int3_selected(_drawLineNr, _label_pstr, (MSG_NOZZLE), & target_temperature[0], 0, HEATER_0_MAXTEMP - 10);
 
lcd_implementation_drawmenu_setting_edit_int3(_drawLineNr, _label_pstr, (MSG_NOZZLE), & target_temperature[0], 0, HEATER_0_MAXTEMP - 10);
 
menu_action_setting_edit_int3((MSG_NOZZLE), & target_temperature[0], 0, HEATER_0_MAXTEMP - 10);

Encore des macros :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
#define lcd_implementation_drawmenu_setting_edit_int3_selected(row, pstr, pstr2, data, minValue, maxValue) lcd_implementation_drawmenu_setting_edit_generic(row, pstr, '>', itostr3(*(data)))
#define lcd_implementation_drawmenu_setting_edit_int3(row, pstr, pstr2, data, minValue, maxValue) lcd_implementation_drawmenu_setting_edit_generic(row, pstr, ' ', itostr3(*(data)))

Voici la fonction appelée, ouf :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
static void lcd_implementation_drawmenu_setting_edit_generic(uint8_t row, const char* pstr, char pre_char, char* data)
{
    char c;
    //Use all characters in narrow LCDs
  #if LCD_WIDTH < 20
      uint8_t n = LCD_WIDTH - 1 - 1 - strlen(data);
    #else
      uint8_t n = LCD_WIDTH - 1 - 2 - strlen(data);
  #endif
    lcd.setCursor(0, row);
    lcd.print(pre_char);
    while( ((c = pgm_read_byte(pstr)) != '\0') && (n>0) )
    {
        lcd.print(c);
        pstr++;
        n--;
    }
    lcd.print(':');
    while(n--)
        lcd.print(' ');
    lcd.print(data);
}

Cela ne gère que l'affichage, partie que je souhaite modifier se trouve dans la 3e fonction
Je suis plus dubitatif...
Je ne trouve que cette déclaration statique, sans code :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
/* Different types of actions that can be used in menu items. */
static void menu_action_back(menuFunc_t data);
#define menu_action_back_RAM menu_action_back
static void menu_action_submenu(menuFunc_t data);
static void menu_action_gcode(const char* pgcode);
static void menu_action_function(menuFunc_t data);
static void menu_action_setlang(unsigned char lang);
static void menu_action_sdfile(const char* filename, char* longFilename);
static void menu_action_sddirectory(const char* filename, char* longFilename);
static void menu_action_setting_edit_int3(const char* pstr, int* ptr, int minValue, int maxValue);
#if 0
static void menu_action_setting_edit_bool(const char* pstr, bool* ptr);
static void menu_action_setting_edit_float3(const char* pstr, float* ptr, float minValue, float maxValue);
static void menu_action_setting_edit_float32(const char* pstr, float* ptr, float minValue, float maxValue);
static void menu_action_setting_edit_float43(const char* pstr, float* ptr, float minValue, float maxValue);
static void menu_action_setting_edit_float5(const char* pstr, float* ptr, float minValue, float maxValue);
static void menu_action_setting_edit_float51(const char* pstr, float* ptr, float minValue, float maxValue);
static void menu_action_setting_edit_float52(const char* pstr, float* ptr, float minValue, float maxValue);
static void menu_action_setting_edit_long5(const char* pstr, unsigned long* ptr, unsigned long minValue, unsigned long maxValue);
#endif
 
#if 0
static void menu_action_setting_edit_callback_bool(const char* pstr, bool* ptr, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_int3(const char* pstr, int* ptr, int minValue, int maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_float3(const char* pstr, float* ptr, float minValue, float maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_float32(const char* pstr, float* ptr, float minValue, float maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_float43(const char* pstr, float* ptr, float minValue, float maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_float5(const char* pstr, float* ptr, float minValue, float maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_float51(const char* pstr, float* ptr, float minValue, float maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_float52(const char* pstr, float* ptr, float minValue, float maxValue, menuFunc_t callbackFunc);
static void menu_action_setting_edit_callback_long5(const char* pstr, unsigned long* ptr, unsigned long minValue, unsigned long maxValue, menuFunc_t callbackFunc);
#endif

C'est le code de UltraLcd - il est blindé de macros

Voici le code source complet du Firmware :
https://drive.google.com/file/d/1PGW...ew?usp=sharing

A bientôt