Stuck on testing Ethernet Class #14
Conversation
src/Devices/Ethernet_TC.cpp
Outdated
| #include "Ethernet_TC.h" | ||
| using namespace std; | ||
|
|
||
| TC_ethernet *TC_ethernet::_instance = nullptr; |
There was a problem hiding this comment.
Could you change the name of the class to match the filename? Please use Ethernet_TC.
Remove extra spaces at end of last line.
src/Devices/Ethernet_TC.cpp
Outdated
| @@ -0,0 +1,32 @@ | |||
| #include "Ethernet_TC.h" | |||
| using namespace std; | |||
There was a problem hiding this comment.
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::.
src/Devices/Ethernet_TC.cpp
Outdated
| // if (current_millis - previous_lease >= LEASE_INTERVAL) { | ||
| Ethernet.maintain(); | ||
| // } | ||
| } |
src/Devices/Ethernet_TC.cpp
Outdated
| } | ||
|
|
||
| void TC_ethernet::renewDHCPLease() { | ||
| // unsigned long current_millis = millis(); |
There was a problem hiding this comment.
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).
| TC_ethernet::TC_ethernet() { | ||
| pinMode(4, OUTPUT); | ||
| digitalWrite(4, HIGH); | ||
| if (Ethernet.begin(mac) == 0) { |
There was a problem hiding this comment.
If the begin() method is successful, does IP get initialized?
There was a problem hiding this comment.
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
src/Devices/Ethernet_TC.cpp
Outdated
| } | ||
|
|
||
| defaultIP = IPAddress(192, 168, 1, 2); | ||
| time_serverIP = IPAddress(132, 163, 97, 1); |
There was a problem hiding this comment.
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.
|
|
||
| #include "Ethernet_TC.h" | ||
|
|
||
| unittest(test) { |
There was a problem hiding this comment.
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.
|
Did you intend to close this? Did this happen when we added high-level tests instead? |
Created a wrapper class for the base Ethernet class in the Arduino Ethernet library. Stuck on issue Arduino-CI/arduino_ci#183.