Skip to content

Commit

Permalink
fix major bug with EtherCard DHCP: when leaseTime is infinity
Browse files Browse the repository at this point in the history
  • Loading branch information
Ray committed Jun 7, 2015
1 parent 29f3b65 commit cc9100b
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 16 deletions.
2 changes: 1 addition & 1 deletion EtherCard.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ class EtherCard : public Ethernet {
* @param dlen Size of the HTTP (TCP) payload
* @param flags TCP flags
*/
static void httpServerReply_with_flags (uint16_t dlen, uint8_t flags, uint8_t dup=0);
static void httpServerReply_with_flags (uint16_t dlen, uint8_t flags);

/** @brief Acknowledge TCP message
* @todo Is this / should this be private?
Expand Down
2 changes: 0 additions & 2 deletions OpenSprinkler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ ulong OpenSprinkler::ntpsync_lasttime;
ulong OpenSprinkler::checkwt_lasttime;
ulong OpenSprinkler::checkwt_success_lasttime;
ulong OpenSprinkler::network_lasttime;
ulong OpenSprinkler::dhcpnew_lasttime;
ulong OpenSprinkler::external_ip;
byte OpenSprinkler::water_percent_avg;

Expand Down Expand Up @@ -261,7 +260,6 @@ byte OpenSprinkler::start_network() {
lcd_print_line_clear_pgm(PSTR("Connecting..."), 1);

network_lasttime = now();
dhcpnew_lasttime = now();

// new from 2.2: read hardware MAC
if(!read_hardware_mac())
Expand Down
1 change: 0 additions & 1 deletion OpenSprinkler.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ class OpenSprinkler {
static ulong checkwt_lasttime; // time when weather was checked
static ulong checkwt_success_lasttime; // time when weather check was successful
static ulong network_lasttime; // time when network was checked
static ulong dhcpnew_lasttime; // time when dhcp renew was performed
static ulong external_ip; // external ip address
static byte water_percent_avg; // average water percentage over a day

Expand Down
11 changes: 7 additions & 4 deletions dhcp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "EtherCard.h"
#include "net.h"

uint32_t now();
#define gPB ether.buffer

#define DHCP_BOOTREQUEST 1
Expand Down Expand Up @@ -237,7 +238,8 @@ static void process_dhcp_offer (uint16_t len) {
case 58: leaseTime = 0; // option 58 = Renewal Time, 51 = Lease Time
for (byte i = 0; i<4; i++)
leaseTime = (leaseTime << 8) + ptr[i];
leaseTime *= 1000; // milliseconds
//leaseTime *= 1000; // milliseconds // ray: use seconds and not millis
if (leaseTime > 86400L) leaseTime = 86400L;
break;
case 54: EtherCard::copyIp(EtherCard::dhcpip, ptr);
break;
Expand Down Expand Up @@ -332,8 +334,8 @@ void EtherCard::DhcpStateMachine (uint16_t len) {
switch (dhcpState) {

case DHCP_STATE_BOUND:
//!@todo Due to millis() 49 day wrap-around, DHCP renewal may not work as expected
if (millis() >= leaseStart + leaseTime) {
//if (millis() >= leaseStart + leaseTime) { // ray: fix millis() 49-day wrap around issue
if (now() > leaseStart + leaseTime) {
send_dhcp_message();
dhcpState = DHCP_STATE_RENEWING;
stateTimer = millis();
Expand Down Expand Up @@ -366,7 +368,8 @@ void EtherCard::DhcpStateMachine (uint16_t len) {
case DHCP_STATE_RENEWING:
if (dhcp_received_message_type(len, DHCP_ACK)) {
disableBroadcast(true); //Disable broadcast after temporary enable
leaseStart = millis();
//leaseStart = millis();
leaseStart = now(); // ray: use seconds and not millis()
if (gwip[0] != 0) setGwIp(gwip); // why is this? because it initiates an arp request
dhcpState = DHCP_STATE_BOUND;
} else {
Expand Down
5 changes: 2 additions & 3 deletions main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ EthernetClient *m_client = 0;
#define NTP_SYNC_TIMEOUT 86403L // NYP sync timeout, 24 hrs
#define RTC_SYNC_INTERVAL 60 // RTC sync interval, 60 secs
#define CHECK_NETWORK_TIMEOUT 59 // Network checking timeout, 59 secs
#define DHCP_RENEW_TIMEOUT 86417L // DHCP renewal timeout: 24 hrs
#define STAT_UPDATE_TIMEOUT 900 // Statistics update timeout: 15 mins
#define CHECK_WEATHER_TIMEOUT 1801 // Weather check interval: 30 minutes
#define CHECK_WEATHER_SUCCESS_TIMEOUT 86433L // Weather check success interval: 24 hrs
Expand Down Expand Up @@ -985,8 +984,8 @@ void check_network() {
if (os.status.network_fails>=6) {
// mark for safe restart
os.status.safe_reboot = 1;
} else if (os.status.network_fails>2 || (now() > os.dhcpnew_lasttime + DHCP_RENEW_TIMEOUT)) {
// if failed more than twice, reconnect
} else if (os.status.network_fails>2) {
// if failed more than twice, try to reconnect
if (os.start_network())
os.status.network_fails=0;
}
Expand Down
2 changes: 1 addition & 1 deletion server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void send_packet(bool final=false) {
if(final) {
ether.httpServerReply_with_flags(bfill.position(), TCP_FLAGS_ACK_V|TCP_FLAGS_FIN_V);
} else {
ether.httpServerReply_with_flags(bfill.position(), TCP_FLAGS_ACK_V, 3);
ether.httpServerReply_with_flags(bfill.position(), TCP_FLAGS_ACK_V);
bfill=ether.tcpOffset();
}
}
Expand Down
8 changes: 4 additions & 4 deletions tcpip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,22 +277,22 @@ void EtherCard::httpServerReplyAck () {
get_seq(); //get the sequence number of packets after an ack from GET
}

void EtherCard::httpServerReply_with_flags (uint16_t dlen, uint8_t flags, uint8_t dup) {
/*void EtherCard::httpServerReply_with_flags (uint16_t dlen, uint8_t flags, uint8_t dup) {
for(byte i=0;i<=dup;i++) {
set_seq();
gPB[TCP_FLAGS_P] = flags; // final packet
make_tcp_ack_with_data_noflags(dlen); // send data
}
SEQ=SEQ+dlen;
}
/*
}*/

void EtherCard::httpServerReply_with_flags (uint16_t dlen , uint8_t flags) {
set_seq();
gPB[TCP_FLAGS_P] = flags; // final packet
make_tcp_ack_with_data_noflags(dlen); // send data
SEQ=SEQ+dlen;
}
*/

void EtherCard::clientIcmpRequest(const uint8_t *destip) {
setMACandIPs(gwmacaddr, destip);
gPB[ETH_TYPE_H_P] = ETHTYPE_IP_H_V;
Expand Down

0 comments on commit cc9100b

Please sign in to comment.