Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Stuck on testing Ethernet Class #14

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .arduino-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ unittest:
- mega2560
libraries:
- "LiquidCrystal"
- "Ethernet"

compile:
platforms:
- mega2560
libraries:
- "LiquidCrystal"
- "Ethernet"
32 changes: 32 additions & 0 deletions src/Devices/Ethernet_TC.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#include "Ethernet_TC.h"
using namespace std;
Copy link
Member

Choose a reason for hiding this comment

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

Are you actually using anything from namespace std or is this just a habit? If you have anything from std then let's reference it directly as std::.


TC_ethernet *TC_ethernet::_instance = nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

Could you change the name of the class to match the filename? Please use Ethernet_TC.


TC_ethernet::TC_ethernet() {
pinMode(4, OUTPUT);
digitalWrite(4, HIGH);
if (Ethernet.begin(mac) == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

If the begin() method is successful, does IP get initialized?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The IP is either initialized through DHCP in the begin(mac) contructor, or (I moved the constructor so it won't be declared unless needed) it will be given a defaultIP with the begin(mac, IP) constructor

Serial.println(F("Failed to configure Ethernet using DHCP"));
Ethernet.begin(mac, defaultIP);
IP = defaultIP;
}

defaultIP = IPAddress(192, 168, 1, 2);
time_serverIP = IPAddress(132, 163, 97, 1);
Copy link
Member

Choose a reason for hiding this comment

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

Does anyone use this?

Copy link
Collaborator Author

@Kavikick Kavikick Nov 5, 2020

Choose a reason for hiding this comment

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

I couldn't find any references in the original implementation for the Time_server. But I'll remove it for now (since I'm not using it), and add it back in later if it's needed. As for defaultIP I moved the declaration to where it is used.

}

TC_ethernet *TC_ethernet::getInstance() {
if (_instance == nullptr) {
_instance = new TC_ethernet;
}
return _instance;
}

void TC_ethernet::renewDHCPLease() {
// unsigned long current_millis = millis();
Copy link
Member

Choose a reason for hiding this comment

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

Either take out the commented-out code or (even better), keep track of the previous lease time and the LEASE_INTERVAL as a private instance variable and constant so that this method can be called frequently and it will decide when it is time to renew (rather than that knowledge being in the caller).


// if (current_millis - previous_lease >= LEASE_INTERVAL) {
Ethernet.maintain();
// }
}
Copy link
Member

Choose a reason for hiding this comment

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

Add newline.

27 changes: 27 additions & 0 deletions src/Devices/Ethernet_TC.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
Based off Arduino's Ethernet library
Implements a wrapper for the Ethernet Class
*/

#pragma once

#include "Ethernet.h"

class TC_ethernet {
public:
static TC_ethernet *getInstance();
IPAddress getIP() {
return IP;
};
void renewDHCPLease();

protected:
TC_ethernet();

private:
static TC_ethernet *_instance;
byte mac[6] = {0x90, 0xA2, 0xDA, 0x00, 0x00, 0x00};
IPAddress defaultIP;
IPAddress time_serverIP;
IPAddress IP;
};
10 changes: 10 additions & 0 deletions test/Ethernet.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#include <Arduino.h>
#include <ArduinoUnitTests.h>

#include "Ethernet_TC.h"

unittest(test) {
Copy link
Member

Choose a reason for hiding this comment

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

It seems that we should be able to do some testing! Get a reference to the singleton, query for the IP, renew the lease, etc.

assertTrue(true);
}

unittest_main()