logic-analyzer icon indicating copy to clipboard operation
logic-analyzer copied to clipboard

new features

Open aster94 opened this issue 6 years ago β€’ 20 comments

@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

  • [x] formatting
  • [x] use of pinMode and digitalWrite outside time-critical task. This will improve readability and won't change the efficency
  • [x] 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
  • [x] use of the LED_BUILTIN for readability and easier port to new boards
  • [x] comment out the debug print statemens
  • [x] 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

aster94 avatar Jan 19 '19 23:01 aster94

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)

sancho11 avatar Jan 22 '19 01:01 sancho11

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 avatar Jan 22 '19 11:01 aster94

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)

aster94 avatar Jan 22 '19 12:01 aster94

Ok, I will do that!

sancho11 avatar Jan 22 '19 12:01 sancho11

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.

sancho11 avatar Jan 26 '19 13:01 sancho11

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

aster94 avatar Jan 26 '19 14:01 aster94

Yeah I do that! the noob way xD

sancho11 avatar Jan 26 '19 14:01 sancho11

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/

aster94 avatar Jan 26 '19 14:01 aster94

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

sancho11 avatar Jan 29 '19 22:01 sancho11

@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'

aster94 avatar Jan 30 '19 23:01 aster94

"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!

sancho11 avatar Feb 02 '19 03:02 sancho11

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!

aster94 avatar Feb 02 '19 10:02 aster94

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

sancho11 avatar Feb 08 '19 07:02 sancho11

#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 avatar Feb 09 '19 19:02 aster94

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!

aster94 avatar Feb 27 '19 18:02 aster94

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!

sancho11 avatar Feb 27 '19 21:02 sancho11

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

aster94 avatar Feb 28 '19 13:02 aster94

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!

sancho11 avatar Mar 01 '19 23:03 sancho11

thanks Sancho, I will write back πŸ˜ƒ

aster94 avatar Mar 02 '19 15:03 aster94

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.

truglodite avatar Jun 06 '19 17:06 truglodite