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

Add PlayCodingBox support: modules, firmware, and drivers #798

Open
wants to merge 1 commit into
base: develop-hw
Choose a base branch
from

Conversation

piaojongwien
Copy link

유의사항

  • base Branch 는 develop-hw 로 통일하였습니다. master, develop 이 아닙니다.
  • 체크박스의 체크는 이슈 생성 후 직접 클릭 or [x] 와 같이 체크하면 됩니다.

어떤 하드웨어 인가요?

[playcodingbox / 플레이코딩 유한책임회사]

어떤 종류의 PR 인가요?

  • [ㅇ ] 신규 하드웨어 추가
  • 기존 하드웨어 변경
  • 기타

이 PR 에서 어떤 부분이 변경되었나요? (작성필요)

  • app/modules/playcodingbox.js: PlayCodingBox의 하드웨어 통신 로직 구현.
  • app/modules/playcodingbox.json: PlayCodingBox의 메타데이터 정의
  • app/modules/playcodingbox.png: PlayCodingBox의 아이콘 이미지 추가.

PR 에 대해 아래 부분을 확인해주세요

  • [ㅇ] 코드 문법 오류가 발생하진 않았는가
  • [ㅇ] 코딩 컨벤션을 정리했는가 (린트, 들여쓰기, 등등..)
  • [ㅇ] base branch 설정을 develop-hw 으로 했는가?
  • [ㅇ] PR 생성 후, 기존 코드와 충돌이 나진 않았나요?

@Tnks2U Tnks2U changed the base branch from master to develop-hw December 19, 2024 05:50
@Tnks2U
Copy link
Contributor

Tnks2U commented Dec 19, 2024

@piaojongwien
타깃 브랜치는 반드시 entrylabs:develop-hw이어야 합니다. 유의 부탁드립니다.

@Tnks2U
Copy link
Contributor

Tnks2U commented Dec 19, 2024

@piaojongwien
PR에 몇가지 문제가 있습니다.
아래의 링크의 변경사항을 참고해서 수정 부탁드립니다.
https://github.com/entrylabs/entry-hw/pull/798/files
검수 및 빌드단계에 들어가서 가급적 빠르게 작업을 부탁드립니다.

  1. app/drivers/CH34x_Install_Windows_v3_4.EXE 파일의 경로가 잘못되었고 사용하고 있지 않습니다.
  • 위 경로에 새로운 드라이버 파일이 추가되었는데, 이 파일을 app/modules/playcodingbox.json 에서 사용하고 있지 않습니다. 또한, 사용하더라도 app/drivers 하위에 바로 위치하는게 아니라, app/drivers/playcoding/CH34x_Install_Windows_v3_4.EXE 와 같이 전용 디렉토리를 만들어 주셔야 합니다.
  1. app/firmwares/examples/arduino_nano/arduino_nano.ino 파일을 수정해선 안됩니다.
  • 위 파일은 arduino nano기반으로 펌웨어를 작성하시는 분들을 위한 예제 파일입니다. 이 파일은 참고용 공용파일이기에 수정해서는 안됩니다.

@piaojongwien
Copy link
Author

요청해주신 사항을 반영하였습니다:

  1. 드라이버 파일을 삭제했습니다.
  2. arduino_nano.ino 파일의 변경 내용을 복구하였습니다.

다시 확인 부탁드립니다. 감사합니다!

@Tnks2U
Copy link
Contributor

Tnks2U commented Dec 19, 2024

@piaojongwien
https://github.com/entrylabs/entry-hw/pull/798/files#diff-867318a44039971af335599e6c99c79f172c9f177d956d0bc32a8ea077200fc8
위 링크나 file changes 탭에서 최종 변경사항을 확인 할 수 있습니다.

  • 드라이버는 위치만 옮겨졌을뿐 그대로 있는 것으로 나옵니다.
  • arduino nano도 여전히 변경사항이 있습니다.

변경작업 후 위 링크를 확인하셔서 재수정을 부탁드립니다.

@piaojongwien
Copy link
Author

다시한번 확인 부탁드립니다.

@Tnks2U
Copy link
Contributor

Tnks2U commented Dec 20, 2024

@piaojongwien
여전히 문제가 있습니다.

  1. app/drivers/CH34x_Install_Windows_v3_4/CH34x_Install_Windows_v3_4.EXE 파일과 app/firmwares/examples/coding_box/coding_box.ino 파일은 기존에 존재하던 파일인데 이 파일을 제거하는 커밋이 포함되어 있습니다.
    file changes에서 위 두 파일의 변경사항이 없어야 합니다.

  2. app/firmwares/examples/arduino_nano/arduino_nano.ino 파일도 개행문자 수정사항이 포함되어 있습니다. 추측하기로 직접 수정사항을 바꾼 뒤에 push하신걸로 보이는데, gitignore 등을 사용해서 변경사항 자체가 없어야 합니다.

제 생각엔, 새로 브랜치를 만든 다음에 필요한 수정사항만 다시 반영해서 새로 PR을 하는게 좋아 보입니다.

@Tnks2U
Copy link
Contributor

Tnks2U commented Dec 20, 2024

@piaojongwien
추가 작업이 없으셔서 공지드린대로 이번달 배포에선 제외됩니다.
요청드린 내용 작업주시면 다음달 배포에 반영하겠습니다.

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