Skip to content

Conversation

@heinrich-foto
Copy link
Contributor

I think a few more actions shortcuts are very helpful. Having to write the complete API URL every time is tedious in my opinion.

Unfortunately I didn't get the JSON validator completely fixed. I still get an error here.

The baseurl I would still have to pack in a config, or further adapt, because also replace and play could be a useful endpoint ...

Currently a small contribution. Thanks for this nice project.

Translated with www.DeepL.com/Translator (free version)

@layereight layereight self-assigned this Jan 19, 2023
@layereight layereight self-requested a review January 19, 2023 08:30
Copy link
Owner

@layereight layereight left a comment

Choose a reason for hiding this comment

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

Hey @heinrich-foto,

I understand that you have a rather big configuration file that contains redundant actions or parts in these actions often recur.

I totally get the problem. Maintaining such a config file can be cumbersome. I also much appreciate the effort you've put into your solution.

But I also have to tell you that the changes you propose introduce a tight coupling to a particular application (Volumio). I'd rather try to avoid that. MFRC522-trigger is designed to be as agnostic as possible to the application it tries to control. In other words it is supposed to be independent of the application it tries to control.

That's why I cannot accept you changes. I'm sorry.

An idea, in order to get what you want, might be to use JSON reference notation ($ref) to reference objects in the same config file. Some commands (like "play", "pause", ...) could be created just once and then be referenced many times in the config file. Other things like playlist (where only the playlist name differs) are harder to achieve in a generic way. Something like "url templates" might be a solution here. E.g.:

{
  "1234567890123": {
    "ondetect": [
      {
        "type": "url-template",
        "name": "playlist"
        "params": [
           "my-playlist-1"
        ]
      }
    ]
  },
  "9876543210987": {
    "ondetect": [
      {
        "type": "url-template",
        "name": "playlist"
        "params": [
           "my-playlist-2"
        ]
      }
    ]
  }
  "url-templates": [
    {
      "name": "playlist"
      "template: "http://localhost:3000/api/v1/commands/?cmd=playplaylist&name={$1}"
    }
  ]
}

But this will also bloat the model for the configuration file and users whould need to understand the concept first. To be honest, I'd rather prefer to keep it simple.

What do you think, @heinrich-foto? Are JSON references ($ref) and/or the concept of "url templates" things that would help and work for you, too?

If so, I would start to look into a solution.



"""
Mayba need to implement a web radio play command
Copy link
Owner

Choose a reason for hiding this comment

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

BTW, this can also be achieved by putting the web radio into a playlist in Volumio.

pathname = path.dirname(path.abspath(__file__))
logging.config.fileConfig(pathname + '/logging.ini')
config = json.load(open(pathname + '/config.json', encoding="utf-8"))
baseurl = "http://localhost:3000/api/v1/commands/"
Copy link
Owner

Choose a reason for hiding this comment

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

This closely ties MFRC522-trigger to a particular application. Something I'd like to avoid.

html = urllib.request.urlopen(url)
html_read = html.read()
j = json.loads(html_read.decode("utf-8"))
if j["Error"]:
Copy link
Owner

Choose a reason for hiding this comment

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

Evaluating the http response is a good idea. But looking for certain properties in the JSON response is a matter of the API contract of the application we're trying to control here. Error and response might be properties that exist in the JSON response of Volumio, but they will certainly not work for other applications.

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