Skip to content
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.

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

Fix misuses read and write pointers #89

Conversation

hagaigold
Copy link
Contributor

Description

Fix misuse of read pointer REG_S3_RX_RD0 & write pointer REG_S3_TX_WR0 for WIZnet shields: W5100 & W5200.
As a result of this fix, tftpPoll function, which processing each new packet, was simplified, and the too-long delay, TFTP_PACKET_DELAY, in each iteration of tftpPoll function was reduced to 5ms (was 400ms).

Fixes #19
Fixes #18

other changes:

  • max address, MAX_ADDR, of code area fixed to match the makefile
  • global (extern) variables definition

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

see issue #19

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings

for W5100 & w5200-  reading data should be from the real physical address.
but when writting back the new value of REG_S3_RX_RD0, it should be without any manipulation-  value should go from 0 to 0xFFFF
for W5100 & w5200-  writing data should be from the real physical address.
but when writting back the new value of REG_S3_TX_WR0, it should be without any manipulation-  value should flow from 0 to 0xFFFF
Copy link
Member

@phillipjohnston phillipjohnston left a comment

Choose a reason for hiding this comment

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

Great work!

#endif

const unsigned char mDebugMain_TITLE[] PROGMEM =
TITLE_BOARD_TYPE " (" TITLE_WIZ_TYPE "), Version " PP_STRINGIFY(BUILD_TAG);
Copy link
Member

Choose a reason for hiding this comment

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

Really like this new approach!

@@ -51,17 +51,13 @@
#define TFTP_PACKET_MAX_SIZE \
(UDP_HEADER_SIZE + TFTP_OPCODE_SIZE + TFTP_BLOCKNO_SIZE + TFTP_MAX_PAYLOAD)

#define TFTP_PACKET_DELAY 400
#define TFTP_PACKET_DELAY 5
Copy link
Member

Choose a reason for hiding this comment

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

Big improvement

@phillipjohnston phillipjohnston merged commit f6f0ea1 into embeddedartistry:master Jan 25, 2022
@hagaigold
Copy link
Contributor Author

Great work!

thx :)

regarding the TFTP_PACKET_DELAY - I did tests with a 0 delay but left a 5 for "just in case" (don't have all the different boards & WIZs)

@phillipjohnston
Copy link
Member

That's great to know, I filed #91 so we can at least give the option to remove it completely

@hagaigold hagaigold deleted the fix/misuses-Read-and-Write-Pointers branch February 4, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate: Ariadne Misuses W5100 and W5200 Performance Improvement: W5500
2 participants