Skip to content
This repository has been archived by the owner on Jun 8, 2023. It is now read-only.

Remove license violations #522

Closed
1 task
kfunaoka opened this issue Jan 25, 2019 · 9 comments
Closed
1 task

Remove license violations #522

kfunaoka opened this issue Jan 25, 2019 · 9 comments

Comments

@kfunaoka
Copy link

kfunaoka commented Jan 25, 2019

Description

Remove license violations

For example, BSD requires:

* Redistributions of source code must retain the above copyright notice, 
  this list of conditions and the following disclaimer.

ToDo

@kfunaoka
Copy link
Author

kfunaoka commented Jan 25, 2019

Anyway, copy/paste is a bad idea.
We should push modifications into upstream.

@kfunaoka
Copy link
Author

@ChenxiTU Would you append Apache license before original BSD Licenses? Your contribution is Apache 2.0 👍

@esteve
Copy link
Contributor

esteve commented Jan 29, 2019

@ChenxiTU I agree with @kfunaoka that copying code from other sources is a bad idea. Can you describe in detail what the changes are and where they are? We should always be aiming to push contributions upstream, maintaining forks ourselves is just adding more work that can be spent elsewhere.

@esteve
Copy link
Contributor

esteve commented Jan 29, 2019

@ChenxiTU I just found autowarefoundation/autoware#1791 (comment) but there's no explanation why you found it hard to control recording from GUI difficult. In any case, you could have inherited from the original code or encapsulated it, instead of copy/pasting it.

@ChenxiTU
Copy link

@esteve The main change is that I added Recorder::stop() and Recorder::start to control the start and stop of recording. I also modify several lines for subscribing topics and add some parameters to control the status. I agree with you, inheriting from the original code can be a better choice. In the further, I will add play function. Let me rewrite recorder core at that time.

@esteve
Copy link
Contributor

esteve commented Jan 29, 2019

@ChenxiTU could you describe the changes in a more detailed way? I want to understand why the original code was difficult to integrate for your use case.

@ChenxiTU
Copy link

@esteve
Add variable recorder_status_ to track the statement of recording
Add variable recorder_error_; to record where and what type of error happened but not shut down the node directly. Some corresponding lines also be modified in like Recorder::checkDuration, Recorder::checkSize(), Recorder::startWriting() etc.
Add variable std::vector<boost::shared_ptr<ros::Subscriber> > subscribers_ to store all sub and make sure all sub will be shut down in Recorder::stop(). Because of this, add one line subscribers_.push_back(sub); in Recorder::subscribe. Without doing this, while recording second or third bags, plugin may crash.
Add method Recorder::start(), Recorder::stop()

@esteve
Copy link
Contributor

esteve commented Jan 30, 2019

@ChenxiTU thank you so much for the detailed explanation, I have a better idea of the changes now.

@gbiggs gbiggs closed this as completed May 29, 2020
@mitsudome-r mitsudome-r transferred this issue from autowarefoundation/autoware Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants