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

merge @jacobrosenthal's fork #2

Open
soldair opened this issue Dec 12, 2014 · 15 comments
Open

merge @jacobrosenthal's fork #2

soldair opened this issue Dec 12, 2014 · 15 comments

Comments

@soldair
Copy link
Collaborator

soldair commented Dec 12, 2014

https://github.com/jacobrosenthal/js-stk500v1

we need to sort out the serial port fork. ideally the interface accepts and open serialport and does not depend on serialport itself. I'm trying to change all of my modules to this interface so you have a good start here accepting the open port in the constructor.

do we really need to issue control signals? or and we just open/close the serialport
stk500 spec doc for ref. http://www.atmel.com/images/doc2591.pdf

i have a problem where the node cli tool depends on 2 dependencies that themselves depend on serialport. npm is not smart enough to share these native deps and each on gets its own copy wasting a bunch of time in compiling. if i want to add this to the cli tool people will have to compile serialport 3 times just to install the cli tool.

@jacobrosenthal
Copy link
Contributor

I got readbootloadcommand and send bootloadcommand merged into my style and have a working Mega example. Its not pretty since were still doing a few things differently and I just wanted a proof of concept before we decided further.
https://github.com/jacobrosenthal/js-stk500v1/tree/v2

I'm not 100% on my javascript yet, but technically I dont think I depend on serial port currently. Its not required within that file anyway. Thats how I swap out for browser serialport right now. Take a look and tell me what would have to change to tie us less directly.

I got to meet the js nodebots community at Robots conf (you were mentioned specifically and I believe missed, btw) and they seem very open. Can we upstream something on serial port to make it not compile several times?

Interesting idea on open close for reset.. Let me know if you test this before me.

@soldair
Copy link
Collaborator Author

soldair commented Dec 13, 2014

im working on stk500v2 also ill check out what you have so far

On Sat, Dec 13, 2014 at 12:16 AM, Jacob Rosenthal notifications@github.com
wrote:

I got readbootloadcommand and send bootloadcommand merged into my style
and have a working Mega example. Its not pretty since were still doing a
few things differently and I just wanted a proof of concept before we
decided further.
https://github.com/jacobrosenthal/js-stk500v1/tree/v2

I'm not 100% on my javascript yet, but technically I dont think I depend
on serial port currently. Its not required within that file anyway. Thats
how I swap out for browser serialport right now. Take a look and tell me
what would have to change to tie us less directly.

I got to meet the js nodebots community at Robots conf (you were mentioned
specifically and I believe missed, btw) and they seem very open. Can we
upstream something on serial port to make it not compile several times?

Interesting idea on open close for reset.. Let me know if you test this
before me.


Reply to this email directly or view it on GitHub
#2 (comment).

@soldair
Copy link
Collaborator Author

soldair commented Dec 13, 2014

Those node bots guys are great. really amazingly helpful and just real good
people im glad you got to hang with them.

Are you keeping support for stk500 v1? I started on 2 because i realized
you implemented v1 but it seems like you replaced the contents with v2.

I just have to copy your pattern of passing it in and we should only
include it as a dev dependency for tests. that will let the implementation
level module be the only one that has the native dep.

Closing the serialport on my linux box still crashes node.
serialPort.close(...)
I have to debug more and pull request a fix. I cant be the only one but for
node bots you dont stop your robot very much. We should totally open a few
pull requests. you with your control signal changes and maybe me if i can
figure out why it crashes.

On Sat, Dec 13, 2014 at 7:18 AM, Ryan Day soldair@gmail.com wrote:

im working on stk500v2 also ill check out what you have so far

On Sat, Dec 13, 2014 at 12:16 AM, Jacob Rosenthal <
notifications@github.com> wrote:

I got readbootloadcommand and send bootloadcommand merged into my style
and have a working Mega example. Its not pretty since were still doing a
few things differently and I just wanted a proof of concept before we
decided further.
https://github.com/jacobrosenthal/js-stk500v1/tree/v2

I'm not 100% on my javascript yet, but technically I dont think I depend
on serial port currently. Its not required within that file anyway. Thats
how I swap out for browser serialport right now. Take a look and tell me
what would have to change to tie us less directly.

I got to meet the js nodebots community at Robots conf (you were
mentioned specifically and I believe missed, btw) and they seem very open.
Can we upstream something on serial port to make it not compile several
times?

Interesting idea on open close for reset.. Let me know if you test this
before me.


Reply to this email directly or view it on GitHub
#2 (comment).

@jacobrosenthal
Copy link
Contributor

Correct. Like I mentioned in email the existing is so coupled and I chose my stack for a reason so I just pulled out the v2 send/receive stuff to see how hard it would be to stick in v1 and managed to get it working. Like I said I think I see the programmer separate and a third project called node dude that ties them together. But what would a programmer interface them look like? What do different protocols require differently. Already see this on stk500. Set options was collapsed into enter programming mode.

Sent from my iPhone

On Dec 13, 2014, at 8:42 AM, Ryan Day notifications@github.com wrote:

Those node bots guys are great. really amazingly helpful and just real good
people im glad you got to hang with them.

Are you keeping support for stk500 v1? I started on 2 because i realized
you implemented v1 but it seems like you replaced the contents with v2.

I just have to copy your pattern of passing it in and we should only
include it as a dev dependency for tests. that will let the implementation
level module be the only one that has the native dep.

Closing the serialport on my linux box still crashes node.
serialPort.close(...)
I have to debug more and pull request a fix. I cant be the only one but for
node bots you dont stop your robot very much. We should totally open a few
pull requests. you with your control signal changes and maybe me if i can
figure out why it crashes.

On Sat, Dec 13, 2014 at 7:18 AM, Ryan Day soldair@gmail.com wrote:

im working on stk500v2 also ill check out what you have so far

On Sat, Dec 13, 2014 at 12:16 AM, Jacob Rosenthal <
notifications@github.com> wrote:

I got readbootloadcommand and send bootloadcommand merged into my style
and have a working Mega example. Its not pretty since were still doing a
few things differently and I just wanted a proof of concept before we
decided further.
https://github.com/jacobrosenthal/js-stk500v1/tree/v2

I'm not 100% on my javascript yet, but technically I dont think I depend
on serial port currently. Its not required within that file anyway. Thats
how I swap out for browser serialport right now. Take a look and tell me
what would have to change to tie us less directly.

I got to meet the js nodebots community at Robots conf (you were
mentioned specifically and I believe missed, btw) and they seem very open.
Can we upstream something on serial port to make it not compile several
times?

Interesting idea on open close for reset.. Let me know if you test this
before me.


Reply to this email directly or view it on GitHub
#2 (comment).


Reply to this email directly or view it on GitHub.

@jacobrosenthal
Copy link
Contributor

Any comments on it?

@soldair
Copy link
Collaborator Author

soldair commented Dec 16, 2014

your stuff looks great. I've start refactoring to make it easier to debug and more robust. I would like your opinion.
the protocol has 2 distinct sections. Sending and receiving generic messages and the implementation of each command.

i have cleaned up the send receive logic and moved it to
lib/parser-v2.js
i have moved the stk500 v2 constants from everywhere to their own file
lib/constants-v2.js
i am working on moving whats left. all of your command implementation to it's own file
stk500-v2.js

the parser and constants could and probably should be their own module just for encapsulation and testing.

I think that your idea for making nodedude a generic programmer is very exciting. starting with stk500 v1 and 2 im happy to help push this forward. We should plan a tree of modules and start publishing.

hopefully ill finish the refactor and open the pull request today. ill make sure i mention you when im ready for code review.

@jacobrosenthal
Copy link
Contributor

Really happy pulled out protocol wrapper out of stk500 and also implemented errors, both on my list
but feels like errors should be unwrapped by parser as well. I was doing 'matchReceive' on mine to try to accomplish this.. not perfect but I felt like I wasnt done trying to do something there. I went to a dev friend for advice and after arguing and talking it out for hour+ he wanted to show me how to better modularize my code and better scope my receive buffer and ended up committing this:
https://github.com/jacobrosenthal/js-stk500v1/tree/modules

Its much more where Im trying to take it than your current refactor

Further, since reset was proven (for now) unnecessary I pulled out the connect and disconnect as well which means were fully stream interface instead of node serial port interface.

@jacobrosenthal
Copy link
Contributor

Also more testable, tests included.

@soldair
Copy link
Collaborator Author

soldair commented Dec 17, 2014

looks good! I'm not a fan of mocha or should. more a tape guy but awesome. open the pull request!

@jacobrosenthal
Copy link
Contributor

v1 and v2 are literally different in everything but name so my current thought is for them to stay separate
And its a good example of our 'programmer' interface to have two programmers available
In this way if you did hate the direction this code base was going you could certainly do what you wish with v2 as long as you have whatever we agree upon (right now, a stream that has a .bootload command)

@soldair
Copy link
Collaborator Author

soldair commented Dec 17, 2014

No hate. Ever. But I completely agree that if packages can be split apart
they should be.

I'm going to be using stk500v2 in production so I have experience and
concerns that you won't have with stk500v1 yet so it makes sense that we
will diverge.

The interface will probably need a verify command as well for those that
want to read the whole flash back out and hash it.

What should the stream emit as readable/data. Just log info?
On Dec 17, 2014 11:02 AM, "Jacob Rosenthal" notifications@github.com
wrote:

v1 and v2 are literally different in everything but name so my current
thought is for them to stay separate
And its a good example of our 'programmer' interface to have two
programmers available
In this way if you did hate the direction this code base was going you
could certainly do what you wish with v2 as long as you have whatever we
agree upon (right now, a stream that has a .bootload command)


Reply to this email directly or view it on GitHub
#2 (comment).

@jacobrosenthal
Copy link
Contributor

Hah, good. Just an expression :) Im actually wrote up most of verify last night so thats coming quickly on my end. Then now that its a stream interface I might abandon my work on node browserify for a simpler chrome serial stream implementation I found (which I cant seem to find right now) We should probably try to agree on tests for sure though :)

@jacobrosenthal
Copy link
Contributor

@jacobrosenthal
Copy link
Contributor

verify implemented and flushed out bootload convenience function that takes a 'board' object. I'm sure there will be a lot of work defining that object (can probably learn a lot by taking from avrdude, theyve been at this a while).

https://github.com/jacobrosenthal/js-stk500v1/tree/modules

The example is kinda ugly now as its seemingly a lot of boilerplate to get to the single command, programmer.bootload..

@soldair
Copy link
Collaborator Author

soldair commented Dec 22, 2014

ill check it out today i hope. this is cool

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

2 participants