Skip to content
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

Ajout prod compteur linky #10

Merged
merged 12 commits into from
Nov 15, 2023
Merged

Ajout prod compteur linky #10

merged 12 commits into from
Nov 15, 2023

Conversation

cddu33
Copy link
Contributor

@cddu33 cddu33 commented Nov 10, 2023

Ajout de la récupération des données de productions du compteur linky
Modification des intitulés prm pour prod et conso
Modification de es intitulés token pour prod et conso
Rajout d'un oui/non pour activer les données de protuction ou non

Copy link
Owner

@bokub bokub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, merci pour ta PR !

Je n'ai pas eu le temps de tout lire car je suis en déplacement mais j'ai déjà laissé quelques premiers commentaires

config.yaml Outdated Show resolved Hide resolved
src/config.ts Outdated
prm: string;
token: string;
name: string;
action: 'yes' | 'non';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pas de mélange français anglais stp

src/index.ts Outdated

const isSyncingNeeded = dayjs(lastStatistic.start).isBefore(dayjs().subtract(2, 'days')) && dayjs().hour() >= 6;
if (!isSyncingNeeded) {
let isSyncingNeeded1 = false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pas fou comme nom de variable

@cddu33
Copy link
Contributor Author

cddu33 commented Nov 11, 2023

Bonsoir, "je vous ai compris " ;)

les modifications demandées sont disponible dans mon dernier PR

@cddu33 cddu33 requested a review from bokub November 11, 2023 15:37
Copy link
Owner

@bokub bokub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci encore pour tes changements, mais y'a pas mal de points qui m'embêtent, désolé pour les allers-retours je suis assez pris en ce moment je n'ai pas pu tout voir au premier passage

Bonsoir, "je vous ai compris " ;)

Si c'est une allusion à Charles de Gaulle, je suis pas sûr d'avoir compris ce que ça vient faire là 🤷

src/config.ts Outdated Show resolved Hide resolved
config.yaml Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
const firstDay = dayjs(lastStatisticP.start).add(1, 'day');
const energyData = await productionClient.getEnergyData(firstDay, true);
incrementSums(energyData, lastStatisticP.sum);
await haClient.saveStatistics(userConfig.production.prm + 'p', userConfig.production.name, energyData);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je pense que ce serait plus clean d'ajouter un paramètre "isProd" dans les fonctions de "haClient".

Dans le cas de la production, on pourrait remplacer l'id linky:12345 par linky_production:12345 ou un truc du genre

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

je l'avais créé dans ma première version mais pas vraiment d'intêret

src/index.ts Show resolved Hide resolved
Comment on lines +52 to +54
history.unshift(LinkyClient.formatLoadCurve(loadCurve));
debug(`Successfully retrieved load curve from ${from} to ${to}`);
offset += interval;
Copy link
Owner

@bokub bokub Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dommage de tout dupliquer au lieu de factoriser

Idem pour la fonction sync où tout est écrit en double

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

je ne connais pas assez HA, donc n'hésite pas à modifier

src/linky.ts Outdated Show resolved Hide resolved
src/linky.ts Outdated Show resolved Hide resolved
@bokub
Copy link
Owner

bokub commented Nov 14, 2023

Merci pour toutes ces modifications !

J'essaie de libérer un peu de temps dans les semaines à venir pour faire quelques tests et modifications mais je ne te promets rien 🤞

@cddu33
Copy link
Contributor Author

cddu33 commented Nov 14, 2023 via email

Copy link
Owner

@bokub bokub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je décommente l'image pour vérifier que ça builde bien dans la CI

config.yaml Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
@bokub bokub changed the base branch from master to feat/production November 15, 2023 06:31
@bokub bokub merged commit 7f347ce into bokub:feat/production Nov 15, 2023
4 checks passed
@bokub bokub mentioned this pull request Nov 15, 2023
bokub pushed a commit that referenced this pull request Nov 15, 2023
@bokub
Copy link
Owner

bokub commented Nov 15, 2023

@cddu33 C'est bon, j'ai mergé ce matin + ajouté quelques améliorations et pas mal de refactorisation (cf #11) 🥳

Je vais faire une release dans la journée. Est-ce que c'est toi cet utilisateur sur HACF ?

@cddu33
Copy link
Contributor Author

cddu33 commented Nov 15, 2023

Yep

@bokub
Copy link
Owner

bokub commented Nov 15, 2023

Super, je ferai également une annonce car beaucoup de personnes attendent cette fonctionnalité. Merci encore pour ton aide !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants