Skip to content

Display manufacturer name from the PID files to make life easier during RDM testing#1237

Merged
peternewman merged 33 commits into
OpenLightingProject:masterfrom
peternewman:plugfest
Nov 20, 2017
Merged

Display manufacturer name from the PID files to make life easier during RDM testing#1237
peternewman merged 33 commits into
OpenLightingProject:masterfrom
peternewman:plugfest

Conversation

@peternewman
Copy link
Copy Markdown
Member

@peternewman peternewman commented Apr 23, 2017

  • Display manufacturer name from the PID files to make life easier during testing

Still need to:

  • Decide if we store all the manufacturer names in the existing manufacturer_pids.proto or in a separate manufacturer_names.proto (same structure, no PIDs) Gone for separate file for potential different update frequencies and to avoid loading when not required
  • Possibly tidy up the code a bit
  • Update RDM-app to do so
  • Fix up PID store loading
  • Ensure it behaves the same when you do a discovery from the RDM test server - Ensure we populate the named UIDs in all circumstances #1342
  • Allow access to the names from C++
  • Add to web UI?
  • Add to CLI tools?

@peternewman peternewman added this to the 0.11.0 milestone Apr 23, 2017
@peternewman peternewman changed the title More minor RDM test fixes More RDM test fixes and usability enhancements Apr 23, 2017
@peternewman peternewman mentioned this pull request Aug 14, 2017
@peternewman peternewman changed the title More RDM test fixes and usability enhancements Display manufacturer name from the PID files to make life easier during RDM testing Aug 15, 2017
@peternewman
Copy link
Copy Markdown
Member Author

This probably wants a more traditional review please @nomis52 . Any thoughts on the questions at the top welcome too, i.e. manufacturer_pids.proto or a separate manufacturer_names.proto?

The current JS changes are designed to be backwards compatible, but we could just replace the old drop down data, at the risk of breaking if someone ends up running a new UI pointing at an old RDM test server, or vice versa.

Comment thread data/rdm/download.sh

echo "Fetching PID data from $datahost"

curl -o pids.proto -f http://$datahost/download?pids=esta
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change is because curl doesn't overwrite the file if the get fails for some reason.

response.SetStatus(HTTPResponse.OK)
return {
'uids': [str(u) for u in uids],
'nameduids': dict(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is to keep compatibility with mismatched JS and Python ends.

$.each(uids, function(item) {
devices_list.append($('<option />').val(uids[item]).text(uids[item]));
});
if (data['nameduids'] != undefined) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is to keep compatibility with mismatched JS and Python ends.

@peternewman peternewman requested a review from nomis52 September 10, 2017 22:04
@peternewman peternewman removed the request for review from nomis52 September 11, 2017 05:39
@peternewman
Copy link
Copy Markdown
Member Author

This is probably a good point for a review and merge @nomis52 . It should load (and ignore) the manufacturer names in C++ currently, but they work as intended in Python and the RDM test stuff. The other stuff can follow in a future PR.

@peternewman peternewman requested a review from nomis52 September 19, 2017 03:13
@peternewman peternewman requested review from nomis52 and removed request for nomis52 November 19, 2017 12:14
@peternewman
Copy link
Copy Markdown
Member Author

Thanks @nomis52 . I'll get this in and address the outstanding tasks in a future PR.

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