-
Notifications
You must be signed in to change notification settings - Fork 491
initial json support #43
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,50 +24,59 @@ | |
#include <ESP8266WiFi.h> | ||
#include <WiFiClientSecure.h> | ||
#include <ESP8266HTTPClient.h> | ||
#include <ArduinoJson.h> | ||
|
||
// FirebaseError represents a Firebase API error with a code and a | ||
// message. | ||
class FirebaseError { | ||
// FirebaseError is an error for a given firebase API call. | ||
struct FirebaseError { | ||
FirebaseError() {} | ||
FirebaseError(int c, const String& m) : code(c), message(m) {} | ||
FirebaseError(int c, const char* m) : code(c), message(m) {} | ||
int code = 0; | ||
String message = ""; | ||
operator bool() const { | ||
return code < 0; | ||
} | ||
}; | ||
|
||
// FirebaseObject is a payload or a result for a given firebase API call. | ||
class FirebaseObject { | ||
public: | ||
operator bool() const { return _code < 0; } | ||
int code() const { return _code; } | ||
const String& message() const { return _message; } | ||
void reset() { set(0, ""); } | ||
void set(int code, const String& message) { | ||
_code = code; | ||
_message = message; | ||
FirebaseObject() : json(_buf.createObject()) { | ||
} | ||
FirebaseObject(const String& data) : _data(data), json(_buf.parseObject((char*)_data.c_str())) { | ||
if (!json.success()) { | ||
error = FirebaseError{-1, "error parsing json"}; | ||
} | ||
} | ||
FirebaseObject(const FirebaseError& err) : error{err}, json(_buf.createObject()) { | ||
} | ||
private: | ||
int _code = 0; | ||
String _message = ""; | ||
StaticJsonBuffer<200> _buf; | ||
String _data; | ||
public: | ||
FirebaseError error; | ||
JsonObject& json; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really want to expose this? It seems like we are exposing internal state. It is a mutable object that the client can change accidentally, when we don't actually need mutability. In addition the client needs to understand the FirebaseObject and the JasonObject. The latter is especially extra work as it involves hunting down the source for another library. What if we keep this internal and expose the get, is, and operator[] in a immutable way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I agree. I took a shortcut because I didn't want to duplicate the API of Also nesting the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. But still need to also forward the cast operators for string and number literal (and get/is). |
||
}; | ||
|
||
// Firebase is the connection to firebase. | ||
// Firebase is a client for a given firebase host. | ||
class Firebase { | ||
public: | ||
Firebase(const String& host); | ||
Firebase& auth(const String& auth); | ||
const FirebaseError& error() const { | ||
return _error; | ||
} | ||
String get(const String& path); | ||
String push(const String& path, const String& value); | ||
FirebaseObject get(const String& path); | ||
FirebaseObject push(const String& path, const FirebaseObject& value); | ||
bool connected(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still feel like the concept of being connected or available is only relevant to one usecase (the stream call) so it seems like this should all be separated from the Firebase object to keep it as simple as possible to understand for the other uses. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, for
|
||
Firebase& stream(const String& path); | ||
FirebaseObject stream(const String& path); | ||
bool available(); | ||
enum Event { | ||
UNKNOWN, | ||
PUT, | ||
PATCH | ||
}; | ||
Event read(String& event); | ||
FirebaseObject read(); | ||
FirebaseObject create(); | ||
private: | ||
String makeURL(const String& path); | ||
String sendRequest(const char* method, const String& path, const String& value = ""); | ||
FirebaseObject sendRequest(const char* method, const String& path, const String& value = ""); | ||
HTTPClient _http; | ||
String _host; | ||
String _auth; | ||
FirebaseError _error; | ||
}; | ||
|
||
|
||
#endif // firebase_h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to below It seems preferable to keep error private as well and expose it as error(). While currently there is no hard reason for this one it will give us much more flexibility in the future without breaking existing implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.