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

paraCada (foreach) #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

paraCada (foreach) #26

wants to merge 1 commit into from

Conversation

renlopes
Copy link
Contributor

Como requisitado no issue #21 essa é uma tentativa de implementar o paraCada (foreach) na biblioteca. Testes de compilação foram positivos no UNO, MEGA e NANO.

  • PS: Necessita de testes práticos.
  • PS2: Pode causar instabilidades se não usada com cuidado pelo consumo de recursos de memória. Talvez explicar tal fato na documentação pode ser relevante.

@OtacilioN
Copy link
Owner

Haha, vou precisar desenterrar minhas skills de C para poder analisar esse código, já vi logo de cara que algumas coisas do código eu simplesmente não sei o que fazem, então vou pesquisar e te perguntar algumas coisas por aqui também, certo?

Uma outra coisa, talvez esse código assuste um pouco algum inciante (Ou até mesmo para gente com certa experiência) então talvez seja válido colocar num local não tão a vista, e de preferência com um aviso de que o código a seguir é complexo. Talvez seja válido colocar esse pedaço de código até em um header a parte, algo do tipo "ComandosAvancados.h"

Contudo a contribuição do foreach é algo importante e que sempre quisemos ter no Brasilino, conforme mapeado na #21 então iremos analisar com bastante carinho e considerar a melhor forma de adicionarmos isto, aproveito e marco o @ErickSimoes como reviwer também.

@renlopes
Copy link
Contributor Author

Em teoria o header (biblioteca) já seria transparente ao usuário. Ele só precisa saber como usar as funções.

@OtacilioN
Copy link
Owner

Sim, para o uso já seria transparente sim, mas é interessante que a Brasilino também seja facilmente "entendível" e "modificável" por qualquer um, inclusive iniciante, para que os mesmos possam contribuir.

@renlopes
Copy link
Contributor Author

Não vejo problema quanto a isso. Estou mais preocupado quanto a capacidade dos controladores aceitarem esse alto nivel já que não estou com meios de teste. Ao menos a compilação na ide correu sem erros pros sockets citados.

@ErickSimoes
Copy link
Collaborator

Oi @alessonrenato!

Que ótima sua contribuição!

Como @OtacilioN comentou, você fez uso de recursos bem avançados (e isso é ótimo). Na época que eu tentei desenvolver essa feature senti dificuldades exatamente por isso. Vou precisar olhar com calma pra tentar entender e avaliar essa questão levantada por Otacílio.

Em breve vou descrever alguns parâmetros de testes para conseguirmos ter certeza que está tudo certo na maior quantidade de plataformas possíveis.

Muito obrigado!

@ErickSimoes
Copy link
Collaborator

ErickSimoes commented May 6, 2018

Eu observei que não foram implementados exemplos do uso do paraCada.
Esses exemplos são muito importantes para os usuários da biblioteca, pois serve como referência, e servirá também como código de testes para validarmos a implementação.

Outro ponto importante é que, tanto para fins didáticos, como boa prática de implementação do projeto, os commits de correção sejam "escondidos", mesclando-os com o commit que ele pretende corrigir. Exemplo, todos os commits depois de 'foreach' são correções dele, como 'localisation: FOREACH-paraCada', 'indentacao e comentarios ' e todos os seguintes. Eles devem ser mesclados no primeiro, já que não adicionam novas funcionalidades.

Quanto aos testes, tendo sido feitas as correções a cima, recomendo testarmos nessas plataformas:

  • Arduino Uno (ATmega328P)
  • Arduino Nano (ATmega328)
  • Arduino Mega (ATmega2560)
  • Arduino Due (AT91SAM3X8E)
  • Arduino Leonardo (ATmega32u4)
  • Franzininho (ATtiny85)

Tendo sido feitos os testes, adicionamos o paraCada na documentação e ele estará pronto para o mundo.

@renlopes renlopes force-pushed the master branch 2 times, most recently from 6d7f9dd to e9bd046 Compare February 12, 2019 05:54
@renlopes
Copy link
Contributor Author

@OtacilioN @ErickSimoes implementei as alterações pedidas, necessita de novos testes.

@ErickSimoes
Copy link
Collaborator

Oi @alessonrenato!

Vamos realizar os testes!

@renlopes renlopes force-pushed the master branch 2 times, most recently from 79a0735 to aab0c80 Compare February 16, 2019 22:51
@renlopes
Copy link
Contributor Author

@ErickSimoes fiz algumas correções e ajustes no arquivo de exemplo para colocar na pasta Basicos, também recoloquei um commit que tinha atropelado do repositorio original após os rebases.

@renlopes
Copy link
Contributor Author

@OtacilioN @ErickSimoes atualizei o PR pra a versão atual do Upstream e corrigi um erro no arquivo de exemplo.

@renlopes
Copy link
Contributor Author

@OtacilioN @ErickSimoes alguma atualização?

@renlopes
Copy link
Contributor Author

renlopes commented Nov 1, 2019

@OtacilioN @ErickSimoes vcs ainda querem a funcionalidade?

@SteffanoP
Copy link
Contributor

SteffanoP commented Apr 7, 2021

Olá @alessonrenato, foi implantada uma Action que realiza o compile para as placas suportadas por este repositório, creio que se você atualizar esta PR com a branch master será possível finalizar a implantação do comando paraCada().

@renlopes
Copy link
Contributor Author

renlopes commented Apr 7, 2021

Faz tanto tempo que esse PR segue pendente de revisão pelos mantenedores que nem sei se tenho habilidade pra portar esse código de baixo nível hoje mais. Não trabalho mais tanto com compilações de código e precisaria relembrar muitos conceitos. Eu lembro que ja na época escrevi todo código com arduíno CLI no konsole e VIM porque já não gostava da IDE. Usei ele pra fazer testes nas plataformas da época e fisicamente testei no UNO, NANO e MEGA que eram as que eu tinha acesso. Não estavam populares plataformas ESP ainda.
Enfim. Se alguém se interessar em atualizar e por os créditos pode ficar a vontade.

@SteffanoP
Copy link
Contributor

Faz tanto tempo que esse PR segue pendente de revisão pelos mantenedores que nem sei se tenho habilidade pra portar esse código de baixo nível hoje mais. Não trabalho mais tanto com compilações de código e precisaria relembrar muitos conceitos. Eu lembro que ja na época escrevi todo código com arduíno CLI no konsole e VIM porque já não gostava da IDE. Usei ele pra fazer testes nas plataformas da época e fisicamente testei no UNO, NANO e MEGA que eram as que eu tinha acesso. Não estavam populares plataformas ESP ainda.
Enfim. Se alguém se interessar em atualizar e por os créditos pode ficar a vontade.

@alessonrenato Você se importa se eu criar uma nova Pull Request, e trazendo o commit 922df92 para ela?

A ideia é que, dessa forma, eu consigo criar um Merge commit da branch master atual junto com o commit 922df92. Porém você também pode fazer isso sem grandes dificuldades.

@renlopes
Copy link
Contributor Author

Pode fazer, sem problemas. Atualmente não to usando as ferramentas do Git.

@SteffanoP SteffanoP mentioned this pull request Apr 12, 2021
@SteffanoP
Copy link
Contributor

@alessonrenato criei uma nova Pull Request (#74), mantive o commit (922df92), para continuar a implementação com a versão mais recente do repositório. No momento que você autorizar, irei abrir a Pull Request para Revisão dos mantenedores/colaboradores. Também estarei aberto para suas opiniões, caso queira continuar com a implementação, tal como de comentários da comunidade.

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.

4 participants