search

srobo / j5

j5 - Framework for Robotics APIs
MIT License
9 stars 4 forks source link

Component-level logging implementation uses a lot of black magic #370

Open kierdavis opened 5 years ago

kierdavis commented 5 years ago

From #369:

This code contains copious amounts of black magic. Perhaps manually adding log messages to every interface method would have been a more obvious if less efficient solution.

We should review whether this implementation is too complex/unclear to be maintainable.

trickeydan commented 5 years ago

Also worth noting that I didn't see this working once during the summer school.

kierdavis commented 5 years ago

@trickeydan were you using Robot(debug=True)? It (deliberately) won't be visible if not.

I've just checked that it still works by adding logging.basicConfig(level=logging.DEBUG) to one of the tests_hw scripts in this repo. Of course, it could be something in the sbot Robot constructor that's causing the logging to not get configured correctly.

trickeydan commented 5 years ago

I was using Robot(debug=True), we should test this specifically with the sbot implementation

kierdavis commented 5 years ago

we should test this specifically with the sbot implementation

We should. I foolishly didn't bring any dev kit home with me, so someone else will have to do the honours :/

kierdavis commented 5 years ago

Are you sure? I tried my best to make sure the dev kit box stayed in Southampton, and the inventory agrees:

[kier@saelli:~/checkouts/srobo/inventory]$ ls southampton/minibots-cupboards/box-18l-rub-sr1XL2A/
info                       motor-board-mcv4b-sr0LK12  motor-board-mcv4b-sr0VJ1K  odroid-u3-sr1TJ7A          servo-board-sbv4b-sr0GJ37  servo-board-sbv4b-sr0RH3N  webcam-logitech-c270-sr2WT25  webcam-logitech-c500-sr1N16
motor-board-mcv4b-sr0GK1A  motor-board-mcv4b-sr0NAN   motor-board-mcv4b-sr0XK1E  odroid-u3-sr1YH71          servo-board-sbv4b-sr0HJ35  servo-board-sbv4b-sr0UH3J  webcam-logitech-c270-srEP37   webcam-logitech-c500-sr2H7W
motor-board-mcv4b-sr0GL19  motor-board-mcv4b-sr0QJ1U  motor-board-mcv4b-sr0YK1C  power-board-pbv4b-sr0KQ2W  servo-board-sbv4b-sr0KH32  servo-board-sbv4b-sr0XH3C  webcam-logitech-c270-srEX30   webcam-logitech-c500-srM1K94
motor-board-mcv4b-sr0HL17  motor-board-mcv4b-sr0UJ1M  odroid-u3-sr1JJ7T          power-board-pbv4b-sr0NW2J  servo-board-sbv4b-sr0LQ2U  servo-board-sbv4b-sr0YH3A  webcam-logitech-c270-srEY3Y
motor-board-mcv4b-sr0JK16  motor-board-mcv4b-sr0UK1L  odroid-u3-sr1LH7P          power-board-pbv4b-sr0PW2G  servo-board-sbv4b-sr0PH3T  usb-hub-startech-sr1MQ3N   webcam-logitech-c270-srN1X34

I'd recommend double-checking whether this RUB is in the cupboard or not.

kierdavis commented 5 years ago

I think this getting off-topic.

Regardless, sbot can operate without a servo board, and we can certainly test this feature without one. I don't think this warrants a "blocked" label.