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

Remove unused member #96

Merged
merged 1 commit into from
Nov 8, 2020
Merged

Conversation

pgrawehr
Copy link
Contributor

@pgrawehr pgrawehr commented May 4, 2020

... that eats a lot of RAM.

... that eats a lot of RAM.
@pgrawehr
Copy link
Contributor Author

@soundanalogous Are you accepting PR's on this project still? I'd have quite a bunch of suggestions/fixes that could make it into the library. (i.e. some more memory optimizations, SPI support, etc).

@soundanalogous
Copy link
Member

Which boards have you verified this change on?

@soundanalogous
Copy link
Member

I will consider pull requests if they have been well tested against a variety of architectures that are compatible with Firmata. I want to avoid fixes that have only been tested on one or two boards.

@pgrawehr
Copy link
Contributor Author

pgrawehr commented Nov 7, 2020

I have only tested the build on a nano so far, but this change is a pure code change that cannot really be hardware dependent. I removed a member of a struct (ow_device_info::addr) that was only ever assigned to. The value was never read.

@soundanalogous
Copy link
Member

Took a long look at this today, including the OneWire library this code depends on. Seems safe to remove that member. I'm not sure why it was ever adde in the first place. My only assumption is that an one point there may have been an intent to store the address value that gets assigned within the search function, but that part was never added.

@soundanalogous soundanalogous merged commit f930899 into firmata:master Nov 8, 2020
@soundanalogous
Copy link
Member

In general, I've been trying to step away from this project, so until someone takes over, expect long delays in code reviews unless a change is very trivial.

@pgrawehr
Copy link
Contributor Author

pgrawehr commented Nov 8, 2020

@soundanalogous I've been quite intensively working with firmata recently, especially writting the client library that will make it into https://github.com/dotnet/iot (See dotnet/iot#1039, currently still in progress because of open dependencies and some low-level design discussions). I expect that this will be quite widely used, as part of a package officially supported by Microsofts dotnet team.

Since there are a few proposed extensions that would really bring the project forward (i.e I've already got an SPI implementation), it would be a pitty to let this project starve.

I haven't pushed to much to this repo yet (not least due to the fact that you seemed otherwise busy), but it would be an honour for me to take this over and bring it forward. Of course, I'd be extra-careful not to break stuff.

@soundanalogous
Copy link
Member

@pgrawehr please see this thread regarding getting involved: firmata/arduino#449.

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.

2 participants