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

rqt_robot_dashboard: missing coloured battery status images #40

Closed
bit-pirate opened this issue Aug 20, 2013 · 7 comments

Comments

Projects
None yet
2 participants
@bit-pirate
Copy link
Contributor

commented Aug 20, 2013

in the SVG format for charging levels 60 to 100.
They already exist as PNG.

@ablasdel

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2013

When those were made it was decided the grey worked better than the green. In hind sight just putting both would have been the smart option for re-usability.

If someone would like to edit the existing ones to change the color I would be perfectly happy to add them alongside the originals.

Until they have been submitted I will mark this as an untargeted enhancement

@bit-pirate

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2013

Hm, I like colours! :-) And there is already a lot of grey in the robot_dashboard.

If I get you a pull request with the coloured versions, would you replace the standards with them? If not, then it wouldn't be worth the effort, since we are using the default battery widget in the turtlebot_dashboard (aka kobuki/create dashboard).

@ablasdel

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2013

I can promise they will be included in the package alongside the other svg files. But for the main dashboards we put together we prefered the look of the grey. It shouldn't be too hard to switch which ones the kabuki uses after they are available.

@bit-pirate

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2013

I'd like to avoid code duplication and extra work. So, as long as there is no big interest in the turtlebot community, I will consider this as unimportant. Thx for your quick feedback!

@ablasdel

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2013

No code duplication required. Look at how pr2 dashboard creates its own battery widget. It should be only a few lines of code. If you have any issues contact me and I'll give you a hand.

bit-pirate added a commit to bit-pirate/rqt_robot_plugins that referenced this issue Aug 21, 2013

@bit-pirate

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2013

OK, you won! I added it.

Although you were right about the minimal extra amount of code being required, I had to fix bug #42 to get it all working.

You really know how to get people working for you! ;-)

@ghost ghost assigned ablasdel Aug 21, 2013

@ablasdel

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2013

Awesome! Seriously I get people to work for me in my sleep!

I have looked at the icons and they look great. I had actually sent an email to the guy who made them originally but you got back quicker!

If the icons were a separate pull request I would merge them immediately.
As it is I need to get another rqt developer to sign off on the bug fix code before I merge.

@ablasdel ablasdel closed this in 71d53e6 Aug 21, 2013

bit-pirate added a commit to bit-pirate/rqt_robot_plugins that referenced this issue Aug 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.