Skip to content

Fully disable cloud connection to airgradient server option #278

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Feb 6, 2025
Merged

Fully disable cloud connection to airgradient server option #278

merged 14 commits into from
Feb 6, 2025

Conversation

samuelbles07
Copy link
Collaborator

Changes

  1. disableCloudConnection config added to disable every cloud connection to airgradient server
  2. disableCloudConnection not applied for DIY model
  3. local-server.md updated, describing offlineMode and disableCloudConnection
  4. Adding more verbose logs regarding ignored connection to airgradient server
> Ignore fetch server configuration. Either mode is offline or cloud connection disabled or configurationControl set to local
> Ignore send data to server. Either mode is offline or cloud connection is disabled or post data to server disabled
> firmwareCheckForUpdate: mode is offline or cloud connection disabled, ignored

@samuelbles07
Add config to disableCloudConnection functions
c5c0dae
@samuelbles07
Apply disableCloudConnection
32c78f6 
fetch configuration, post data and ota will be ignored
@samuelbles07
Update docs
1537d5d 
Notes about offlineMode and disableCloudConnection
@samuelbles07
Update diy samples
c8f0e6a 
That accomodate ApiClient changes
Fix apiClient begin on OneOpenAir
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
examples/BASIC/BASIC.ino
"or configurationControl set to local");
apiClient.resetFetchConfigureState();
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ignore vs reset
also propose to log what the mode is

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ignore vs reset

Relate to this comment #278 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Too much redundancy code added to only log the exact mode. I think it's enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The method should start with a verb. e.g.
update ConfigurationSchedule()
it this is what it does. Within the method we then say resetFetchConfigurationStatus() so we have update vs. reset. Not entirely sure how to resolve this.

nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
nick-4711
nick-4711 reviewed Jan 25, 2025
View reviewed changes
@samuelbles07 samuelbles07 changed the base branch from master to develop January 26, 2025 15:18
nick-4711
nick-4711 reviewed Feb 4, 2025
View reviewed changes
docs/local-server.md Outdated
| `monitorDisplayCompensatedValues` | Set the display show the PM value with/without compensate value (only on [3.1.9]()) | Boolean | `false`: Without compensate (default) <br> `true`: with compensate | `{"monitorDisplayCompensatedValues": false }` |
| `corrections` | Sets correction options to display and measurement values on local server response. (version >= [3.1.11]()) | Object | _see corrections section_ | _see corrections section_ |


**Notes**

- `offlineMode` : device will disable all network operation, and only show measurements on display and ledbar; Read-Only; Change can be apply using reset button on boot.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Propose to make full sentences

The device ... on the display ....

examples/BASIC/BASIC.ino Outdated
if (configuration.isPostDataToAirGradient() == false ||
configuration.isOfflineMode()) {
if (configuration.isOfflineMode() || !configuration.isPostDataToAirGradient()) {
Serial.println("Ignore send data to server. Either mode is offline "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Skipping transmission of data to AG server. ?

examples/BASIC/BASIC.ino Outdated
}

if (wifiConnector.isConnected() == false) {
Serial.println("WiFi not connected, skip post data to server");
Copy link
Collaborator

Choose a reason for hiding this comment

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

.. skipping data transmission to AG server
?

src/AgWiFiConnector.cpp Outdated
"<p>Prevent connection to the AirGradient Server. Important: Only enable "
"it if you are sure you don't want to use any AirGradient cloud "
"features. As a result you will not receive automatic firmware updates "
"and your data will not reach the AirGradient dashboard.</p>");
WIFI()->addParameter(&postToAgInfo);
"configure from cloud and your data will not reach the AirGradient dashboard.</p>");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it needs an extra comma here and
configure -> configuration (or maybe configuration settings)

nick-4711
nick-4711 reviewed Feb 5, 2025
View reviewed changes
examples/OneOpenAir/OneOpenAir.ino Outdated
@@ -505,15 +466,21 @@ static bool sgp41Init(void) {

static void firmwareCheckForUpdate(void) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

propose to rename to
checkForFirmwareUpdate

examples/OneOpenAir/OneOpenAir.ino Outdated
// Initialize api client
apiClient.begin();

// Check and process if AirGradient server is reachable
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment doesn't seem to be in line with what we then do, we "send"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

nick-4711
nick-4711 reviewed Feb 5, 2025
View reviewed changes
examples/OneOpenAir/OneOpenAir.ino Outdated
@@ -870,7 +870,7 @@ void initializeNetwork() {
// Initialize api client
apiClient.begin();

// Check and process if AirGradient server is reachable
// Send data for the first time to airgradient at boot
Copy link
Collaborator

Choose a reason for hiding this comment

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

to AG server
?

@samuelbles07 samuelbles07 merged commit 3226c14 into develop Feb 6, 2025
20 checks passed
@samuelbles07 samuelbles07 deleted the feat/disable-cloud branch February 6, 2025 03:13
@samuelbles07 samuelbles07 mentioned this pull request Feb 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nick-4711 nick-4711 nick-4711 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@samuelbles07 @nick-4711