Skip to content

Adding my edit to the code - Hussain#1

Open
HUSSAIN1711 wants to merge 3 commits intozotbins:mainfrom
HUSSAIN1711:main
Open

Adding my edit to the code - Hussain#1
HUSSAIN1711 wants to merge 3 commits intozotbins:mainfrom
HUSSAIN1711:main

Conversation

@HUSSAIN1711
Copy link
Copy Markdown

(only changed one component "TM1637Display1.cpp")

Copy link
Copy Markdown
Collaborator

@alxkeda alxkeda left a comment

Choose a reason for hiding this comment

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

looks fine

  • might consider changing esp_rom_delay_us to a non-blocking vtaskdelay (rtos) because it is called in an rtos task (checkbuttonpressed)
  • can you rename for us the TM1637Display.cpp file to countdown.cpp or something like that since it contains the function definitions for the countdown functionality, not the lib functions for the display (and then you can also change tm1637display1 to tm1637display)
  • edit chad's countdown code to use the display.start() .stop() member functions (probably not needed but for clarity)
  • if its not working, you could quickly test that the pins youre using to interface with the display are working. i was having issues with pins 12/13 when i was getting the ultrasonic sensor to work. pins 2 and 16 worked for me, just make sure there is no overlap between the button input pin
  • on that same note, i believe we are using pin 14 to check the button and i would create some macros for that pin so its a little clearer
  • additionally, we're using 1/3 right now so i would test that they are working, can ask anthony or krish about the debugger or you can just test that things are working with the esp logging component (esp_logi(), etc.). particularly i think it would be useful to log a message to the esp monitor in vscode whenever the button is pressed, that way you can troubleshoot whether the issue is with the button or the countdown code

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.

2 participants