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 spark fun gps #1074

Closed
wants to merge 11 commits into from
Closed

Add spark fun gps #1074

wants to merge 11 commits into from

Conversation

makenai
Copy link
Contributor

@makenai makenai commented Mar 29, 2016

This adds support for the SparkFun Venus GPS module: https://www.sparkfun.com/products/11058

@makenai
Copy link
Contributor Author

makenai commented Mar 29, 2016

I'm not sure what's up with the serialport test failing. Is there anything I need to do to fix that?

@rwaldron
Copy link
Owner

rwaldron commented Apr 1, 2016

No, it's some strange thing that appeared recently :(

lib/gps.js Outdated
* SkyTraq Binary Message Protocol
* http://cdn.sparkfun.com/datasheets/Sensors/GPS/Venus/638/doc/AN0003_v1.4.19.pdf
*/
GPS.prototype.sendSkyTraqCommand = function(payload, callback) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this specific to just this model? There shouldn't be component specific methods like this, so I think you and @dtex should coordinate on this implementation.

@rwaldron rwaldron force-pushed the master branch 4 times, most recently from 7e94449 to aaee114 Compare April 13, 2016 19:16
@rwaldron
Copy link
Owner

@dtex @makenai what's the word?

@dtex
Copy link
Collaborator

dtex commented Apr 18, 2016

It looks like the heavy lifting is done, just some refactoring. How do you (@rwaldron) feel about that proposed PARSER object?

@rwaldron
Copy link
Owner

@dtex but there is still both a sendCommand and sendSkyTraqCommand? Is that going to be fixed with the new parser thing you're describing? I think that sounds like a good solution.

@rwaldron
Copy link
Owner

Also, needs one of these: git pull --rebase upstream master

@dtex
Copy link
Collaborator

dtex commented Apr 18, 2016

sendCommand will vary with the chip but parsers will not so just leave sendCommand on the prototype and override that with a custom sendCommand on CHIP.VENUS638FLPX.

@rwaldron
Copy link
Owner

so just leave sendCommand on the prototype and override that with a custom sendCommand on CHIP.VENUS638FLPX.

Yeah, this is what I wanted. I don't want another sort-of-like-sendCommand-thing.

@rwaldron
Copy link
Owner

@dtex @makenai what's the next step?

@makenai
Copy link
Contributor Author

makenai commented Apr 23, 2016

@rwaldron I can definitely it a shot at that, but it would take me a couple of weeks to get around to it right now. If @dtex or anyone with a stronger idea of how it should work would like to get it started, I'm open to that too!

@dtex
Copy link
Collaborator

dtex commented Apr 25, 2016

I can work on those changes. I've got a pretty good idea of what needs to happen. Is the best way to get this in is to fork makenai:addSparkFunGPS and make a PR to that, or should I just open a new PR here?

@rwaldron
Copy link
Owner

Is the best way to get this in is to fork makenai:addSparkFunGPS and make a PR to that

That's what I would recommend doing :) it will be trivial for @makenai to land it when you're ready

@rwaldron rwaldron force-pushed the master branch 5 times, most recently from beb9597 to 4357638 Compare July 8, 2016 21:29
@dtex
Copy link
Collaborator

dtex commented Aug 2, 2016

Hi @makenai,

Are you back home? NBD is behind us and I wanted to get Tessel GPS support added but we should land this first. Any plans to work on it soon? If not, I could probably get my hands on a Venus and apply the finishing touches.

Your trip looked awesome BTW. I so want to visit there.

@makenai
Copy link
Contributor Author

makenai commented Aug 2, 2016

Yup, I'm back! And this has been weighing heavily on my conscience. :) I'm at CascadiaFest this week, but I'm going to plan to take a look at it during / possibly a bit after depending on my motivation level.

@dtex
Copy link
Collaborator

dtex commented Aug 2, 2016

Okay, have fun at Cascadia. I was there last year and had a really good time.

@rwaldron rwaldron force-pushed the master branch 2 times, most recently from 7d6e95a to 935fd36 Compare August 3, 2016 19:59
@rwaldron
Copy link
Owner

🔔

@dtex
Copy link
Collaborator

dtex commented Aug 24, 2016

@makenai I'm anxious to land support for the Tessel GPS module. I want this ball of cups to be able to drive itself around the block (Halloween is coming and how fun would it be to add some lights and let it go?). I've been holding off because I refactored a lot in my PR to your repo and I don't want to lose that. Also, I'm worried that if I submit my own PR with the Tessel GPS stuff it will make merging a ginormous PIA for you. Do you think you'll have it in the next couple of weeks?

@dtex
Copy link
Collaborator

dtex commented Jan 11, 2018

Hi @makenai,

I hope all is well.

This PR has sat for a while and neither one of us have gotten around to it. I want to close it for cleanliness but I don't want the addition of the Venus to fall off the radar. To handle cases like this I've created a new Requested Features page in the wiki and added it to that list.

@dtex dtex closed this Jan 11, 2018
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.

3 participants