Skip to content

Sine wave pH#269

Merged
jgfoster merged 26 commits intoOpen-Acidification:mainfrom
eucalvo:SlopePH
Dec 22, 2021
Merged

Sine wave pH#269
jgfoster merged 26 commits intoOpen-Acidification:mainfrom
eucalvo:SlopePH

Conversation

@eucalvo
Copy link
Copy Markdown
Collaborator

@eucalvo eucalvo commented Dec 3, 2021

No description provided.

@eucalvo eucalvo requested a review from jgfoster December 3, 2021 17:06
Copy link
Copy Markdown
Member

@prestoncarman prestoncarman left a comment

Choose a reason for hiding this comment

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

The main question is how to deal with naming pH variables and files. I think we need to be more consistent. We seem to use "pH", "PH", and "Ph". I can see arguments for all of these, but in most cases I think that "Ph" is probably the correct answer.
Thought?

Comment thread src/Devices/EEPROM_TC.cpp Outdated
float EEPROM_TC::getRampStartingTemp() {
return eepromReadFloat(RAMP_STARTING_TEMP_ADDRESS);
}
uint16_t EEPROM_TC::getPHSetType() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Ph" or "PH"? Lets be consistent and make it always "Ph" in the function names.

Comment thread src/Devices/EEPROM_TC.cpp Outdated
void EEPROM_TC::setRampStartingTemp(float value) {
eepromWriteFloat(RAMP_STARTING_TEMP_ADDRESS, value);
}
void EEPROM_TC::setPHSetType(uint16_t value) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Ph" or "PH"? Lets be consistent and make it always "Ph" in the function names.

Comment thread src/Devices/EEPROM_TC.h Outdated
float getPHInterval(); // not used
float getPHSeriesPointer(); // not used
float getPHSeriesSize(); // not used
float getPHDelay(); // not used
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Ph" or "PH"? Lets be consistent and make it always "Ph" in the function names. Update for the whole file.

Comment thread test/MenuTest.cpp Outdated
enterKey('D');
}

// 2 goes up in the menu
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this comment would be nice to have as a multiline comment associated with the function.

/**
 * 2 goes up in the menu
 * ...
 */
unittest(MainMenu) {

This should help it to show up with function documentation.

@jgfoster
Copy link
Copy Markdown
Member

jgfoster commented Dec 9, 2021

The chemists and biologists use "pH" as the proper designation, but that doesn't fit very well with camelCase. So, consistency is good and Ph fits so I'd be inclined to go with that...

@jgfoster jgfoster merged commit 92e1ce9 into Open-Acidification:main Dec 22, 2021
@eucalvo eucalvo deleted the SlopePH branch March 3, 2022 21:34
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