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

new features #7

Open
6 of 16 tasks
aster94 opened this issue Jan 19, 2019 · 20 comments
Open
6 of 16 tasks

new features #7

aster94 opened this issue Jan 19, 2019 · 20 comments

Comments

@aster94
Copy link
Owner

aster94 commented Jan 19, 2019

@sancho11

We need to decide which formattation of the code we will keep because it is becoming a mess of whitespaces, wrong indentation ecc. Which IDE are you using for processing and which one for arduino?
I am using vscode for arduino and the default formatting and for processing the default IDE but the default formatting is not very good, maybe is better to use all on vscode this way the formatting between arduino and processing will be the same

I also made a todo list

  • formatting
  • use of pinMode and digitalWrite outside time-critical task. This will improve readability and won't change the efficency
  • the steps of the mouse wheel when we are in a deep zoom is too big, we should change the steps according to the reducer
  • use of the LED_BUILTIN for readability and easier port to new boards
  • comment out the debug print statemens
  • move the button for the times print in the bottom bar
  • move instructiuon in the readMe
  • change the reducer with the mouse wheel
  • auto adjust the size of the screen based on the pin used, maybe too much work, better to focus on other task 😅
  • pass the number of samples directly from processing (this way we won't need to modify the MCU sketches, all the configuration will be in processing)
  • in the code comments use english and add more comments for functions and where needed
  • new images and update readme
  • the first draw is far from the left border
  • clean of the code, slipt of functions, use better names for the variables
  • add custom board and rework pinassigment
  • run on all the boards

What do you think about?
I can work on the communication of the samples number between processing and arduino

@sancho11
Copy link

Hi!, well yeah is a mess xD. I was using Notepad++ because its a ligth text editor with some programing tools, but is not a IDE.
I think that would be wonderful make that improvements.
Just some things to keep in mind. The number of examples define the amount of memory that the board will use when you run the program, so I think that we can´t do that. We could make the asignation of memory in the board code and them say to the board how many samples we want to receive as long as they are less than those defined in the code, and this in turn would depend on the capabilities of the board.
I dont understand what is "comment out the debug print statemens" xD
For the rest I think I quite agree.
So... what IDE you want to use?
And... Where do you think I should start?
Have a nice day! (or night xD)

@aster94
Copy link
Owner Author

aster94 commented Jan 22, 2019

I think that would be wonderful make that improvements.

muy bien 😄

Hi!, well yeah is a mess xD. I was using Notepad++ because its a ligth text editor with some programing tools, but is not a IDE.
So... what IDE you want to use?

ok, if i am not wrong you are using an old computer so maybe VSCode would be too heavy, I will adjust the formattation and push the code, then you could try to follow the existing formattation so you won't need to install more stuff

The number of examples define the amount of memory that the board will use when you run the program, so I think that we can´t do that. We could make the asignation of memory in the board code and them say to the board how many samples we want to receive as long as they are less than those defined in the code, and this in turn would depend on the capabilities of the board.

Exactly, you perfectly got what i was thinking about!

I dont understand what is "comment out the debug print statemens" xD

from this print("something") to //print("something") i already done it, i forgot to mark it 😅

@aster94
Copy link
Owner Author

aster94 commented Jan 22, 2019

ok with commit 82bbeac the formatting is all the same in all the sketches! you can download the new branch

We can split the todo list and do one at the time to avoid merge conflicts. I can work on this:

pass the number of samples directly from processing (this way we won't need to modify the MCU sketches, all the configuration will be in processing)

could you please rewrite some comments in english and choose better names for the cursora, cursoraf and so on? I will do the same for the buttons names and for other variables (i choosed very ugly names ahahah)

@sancho11
Copy link

Ok, I will do that!

@sancho11
Copy link

Yes, github has me a little confused. I know it is an important tool, however I do not know how to use it yet. I did not know how to update my fork when you made the commits, and now that I did commit it does not let me update it. I think I will erase my fork and I will clone it directly from yours I will try to update correctly from now on.

@aster94
Copy link
Owner Author

aster94 commented Jan 26, 2019

ningùn problema! git is quite complex 😫

So could you please confirm me that this is what you did?

you downloaded as a zip file the branch new-features from my repo and from it you started adding the changes then uploaded all in your repo and you made the pull request, correct?

I need to know this because otherwise i need to look for missing commits

@sancho11
Copy link

Yeah I do that! the noob way xD

@aster94
Copy link
Owner Author

aster94 commented Jan 26, 2019

normal man! i did it that way until the last month!

so after you deleted your remote repo and your working folder do something like this:
git clone https://github.com/aster94/logic-analyzer.git

this change the branch
git chekout pr

and to update your remote from your local do
git push

then do a PR from github!

try to to it and add somethin (like a new line) in some file and do a PR!

then to update your remote repo from my remote repo you should be able to do all from github web and then git pull in your local

there is also a more complex method which doiesn't need github web:
https://gist.github.com/CristinaSolana/1885435
https://help.github.com/articles/syncing-a-fork/

@sancho11
Copy link

Hi and thanks, I was able to synchronize the fork!

@aster94
Copy link
Owner Author

aster94 commented Jan 30, 2019

@sancho11 first of all thanks for the changes! it was very good to use small function to draw the channels, this is the correct approach 😎

I started making small changes after lunch, i stopped only now, damned programming 😂
I reworked really a lot of stuff, the most noticeables changes are

  • splitted the code in three parts
  • class Button and Box
  • function mouse_over_channel and mouse_over_button you can see how much of repetitive code these two functions saved us!
  • deleted some ugly names, added more comments

The channel selection part is not working, i am not sure where the error is, the only part i think that i changed was this else because i thought it was in the wrong position, what do you think about it? https://github.com/aster94/logic-analyzer/blob/new-features/Computer_Interface/Computer_Interface.pde#L1035 is it correct? 🤔
or maybe it could be related to some changes to yBottom, xEdge or other strange variables names, I am trying to delete some of them in favour of more meaningful names

anyway do i remember wrong or once the reducer was changed with the mouse wheel instead of the mouse buttons?
do you have any idea why the firsts lines drawn are far from the border? this was something that happened before my changes and i still didn't investigate it

still there is more work to be done especially to make the code more readable, i have difficulties to understand some functions 😂

I think that next time i will rework the pin assignment to make it smaller!

ahhh be careful this time to bring this changes to your repository, check that your pr branch is even with this! right now you should see `This branch is 2 commits behind aster94:pr'

@sancho11
Copy link

sancho11 commented Feb 2, 2019

"anyway do i remember wrong or once the reducer was changed with the mouse wheel instead of the mouse buttons?"
Yes, you got me! I changed that functionality because I think it's more common to need to move forward and back than zoom, so change them.
"do you have any idea why the firsts lines drawn are far from the border? this was something that happened before my changes and i still didn't investigate it"
Yes, that was me too... I add a signal offset in the board code, because I thought it would be convenient to leave this space, to delimit the starting point, because if the signal did not start long after time zero, now it always starts 125 micro seconds (or the value that has the offset defined in the board code), so that it looks a little more orderly. Or so I think.
"ahhh be careful this time to bring this changes to your repository, check that your pr branch is even with this! right now you should see `This branch is 2 commits behind aster94:pr'"
Yes, I will.
"The channel selection part is not working, i am not sure where the error is, the only part i think that i changed was this else because i thought it was in the wrong position, what do you think about it? https://github.com/aster94/logic-analyzer/blob/new-features/Computer_Interface/Computer_Interface.pde#L1035 is it correct? 🤔"
Yes, I will take care of that.

Im glad that you like the changes!

@aster94
Copy link
Owner Author

aster94 commented Feb 2, 2019

Yes, you got me! I changed that functionality because I think it's more common to need to move forward and back than zoom, so change them.

Yes, that was me too... I add a signal offset in the board code, because I thought it would be convenient to leave this space, to delimit the starting point, because if the signal did not start long after time zero, now it always starts 125 micro seconds (or the value that has the offset defined in the board code), so that it looks a little more orderly. Or so I think.

okkkkkk don't worry for now we can leave these changes and think more about them later
Once you fix the channel selector and improve the readability we will decide!

@sancho11
Copy link

sancho11 commented Feb 8, 2019

The drawbacks were solved, for this I had to make minimal changes in some sections of the code.
I have also seen your contributions and I think it is ideal to use classes to sort the data and functions, I like what you get and I will try to apply this methodology in my functions too!
Have a nice day :D

@aster94
Copy link
Owner Author

aster94 commented Feb 9, 2019

#9 merged! great work sancho 😁
I have an exam soon so for a few of weeks I will be unable to make any changes but we are almost at the end!

@aster94
Copy link
Owner Author

aster94 commented Feb 27, 2019

Unfortunately for other duties coming I won't be able to complete the work on this 😥
maybe one day I will complete this, until then all pull request concerning this issue or other are welcomed!

@sancho11
Copy link

sancho11 commented Feb 27, 2019

It's a shame. I hope you do well in what you are doing! I'm also busy but as soon as I can, I'll make more contributions to the project. Take it easy and good luck!

@aster94
Copy link
Owner Author

aster94 commented Feb 28, 2019

thanks sancho! unfortunately I have the italian version of the "MIR examen" so until the summer i would be completely focused on it!

@sancho11
Copy link

sancho11 commented Mar 1, 2019

I'm not from Spain (I'm from Latin America), I had to google it, haha.
I wish you the best of luck, you almost succeed, give the best of you!
Regards!

@aster94
Copy link
Owner Author

aster94 commented Mar 2, 2019

thanks Sancho, I will write back 😃

@truglodite
Copy link

Best of luck aster94, hope you do well!

To all, I've got some time to work on moving the documentation from computer_interface.pde to the readme. I've forked the repo and am keeping up to date with the development branch. I'll submit any PR's from there.

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

No branches or pull requests

3 participants