Little update for lwIP_enc28j60#8376
Little update for lwIP_enc28j60#8376Nourbakhsh-Rad wants to merge 2 commits intoesp8266:masterfrom Nourbakhsh-Rad:patch-2
Conversation
d-a-v
left a comment
d-a-v
left a comment
There was a problem hiding this comment.
Beside the led API addition, some methods are aliased or duplicated.
Can you please enlighten us about your proposal ?
|
|
||
| //-------------------------------------------------------- | ||
| bool | ||
| ENC28J60::resetEther(void) |
There was a problem hiding this comment.
Sometimes the ENC28J60 does not work properly (freezes) and needs to be reconfigured (please GOOGLE "enc28j60 freezes")
There was a problem hiding this comment.
Ah you need to call it from "userland".
Instead of duping it, why don't you move it to the public section ?
| } | ||
|
|
||
| uint8_t | ||
| ENC28J60::hardwareStatus(void) |
There was a problem hiding this comment.
Is it really hardwareStatus() or hardwareRevision/hardwareVersion() ?
We already have ::readrev() which seems to be identical.
There was a problem hiding this comment.
- This function is similar to readrev function (it is private)
- If this function returns 0xFF, that means "Hardware Error" (incorrect hardware revision)
(The EREVID register is commonly used to ensure the health of the hardware)
There was a problem hiding this comment.
This function can be changed this way
uint8_t
ENC28J60::hardwareStatus(void)
{
return readrev(); /* 0xFF = Hardware Error */
}
There was a problem hiding this comment.
Or it could be, as for reset(), moved to the public section without changing its name ?
| // PHLCON: PHY module LED control register | ||
| // | ||
| // R R R R L L L L L L L L R | ||
| // E E E E A A A A B B B B L L S E | ||
| // S S S S C C C C C C C C F F T S | ||
| // E E E E F F F F F F F F R R R E | ||
| // R R R R G G G G G G G G Q Q C R | ||
| // V V V V 3 2 1 0 3 2 1 0 1 0 H V |
There was a problem hiding this comment.
Can this table receive more comments on what it is for ?
There was a problem hiding this comment.
This is LED control register that described in the ENC28J60 document (page 11) and used with LEDsConfig function
usage:
LEDsConfig(LED_LINK_STATUS, LED_TRANS_RECV); // LEDA, LEDB
LEDsConfig(LED_BLINK_SLOW, LED_NO_CHANGE); // LEDA, LEDB
There was a problem hiding this comment.
Do you plan to add this informations as source-code comments in your proposal ?
| // wait until the PHY read completes | ||
| while(readreg(MISTAT) & 0x01){ | ||
| delay(10); | ||
| wdt_reset(); |
There was a problem hiding this comment.
IMHO, delay() takes care of resetting the watchdog.
| // wait until the PHY write completes | ||
| while(readreg(MISTAT) & 0x01){ | ||
| delay(10); | ||
| wdt_reset(); |
There was a problem hiding this comment.
delay implies a WDT reset.
|
Please, while you are updating this lib, could you check about the fate of
I don't see any definition of the function anywhere. |
3 participants
A little update for lwIP_enc28j60 ...