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

Update file naming and macro naming for consistency, add missing notifications for PYLON_CAN and CHADEMO_BATTERY #80

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

lenvm
Copy link
Collaborator

@lenvm lenvm commented Nov 7, 2023

This pull request updates the file names and macros for naming consistency, and it orders macros alphabetically.

Consistency and alphabetical ordering allows easier checking if the code is complete.

Some examples of changes that were made:

  • macro update: CAN_BYD --> BYD_CAN (as all other macros first start with the name of the vendor, followed by the protocol)
  • file name update: MODBUS_LUNA2000.cpp --> LUNA2000_MODBUS.cpp (as other filenames start with the name of the vendor, followed by the name of the protocol)

By ordering the macros alphabetically, I found that:

  • a notification of which inverter is used in case of PYLON_CAN was missing
  • a notification of which battery is used in case of CHADEMO_BATTERY was missing

Both these notifications are now added.

@lenvm lenvm force-pushed the feature/naming-consistency branch from 6ec6b7c to a2aaadf Compare November 7, 2023 22:37
Copy link
Owner

@dalathegreat dalathegreat left a comment

Choose a reason for hiding this comment

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

Great improvement!

Btw lenvm, incase you want to join the Discord server, here is an invite link: https://discord.gg/5dyBXtAb

@dalathegreat dalathegreat merged commit 3e5f72f into dalathegreat:main Nov 8, 2023
1 check passed
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.

2 participants