-
Notifications
You must be signed in to change notification settings - Fork 0
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
various fixes, and idiomatic Go #5
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
95cd785
various fixes, and idiomatic Go
mpl 0ac6857
tiny changes
mpl bf8e937
Use TransactionJSON to avoid unnecessary unmarshal
Bucknalla 9699dd7
Addresses logging changes
Bucknalla 480a9b0
Report failed initialisation over HTTP
Bucknalla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,4 +8,5 @@ services: | |
- "/dev/i2c-1:/dev/i2c-1" | ||
expose: | ||
- "3434" | ||
privileged: true | ||
privileged: true | ||
restart: always |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,107 +1,109 @@ | ||
package main | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"log" | ||
"net/http" | ||
"os" | ||
|
||
"github.com/blues/note-go/notecard" | ||
) | ||
|
||
type Transport string | ||
|
||
const ( | ||
Serial Transport = "serial" | ||
I2C Transport = "i2c" | ||
Serial = "serial" | ||
I2C = "i2c" | ||
) | ||
|
||
var card *notecard.Context | ||
type server struct { | ||
card *notecard.Context | ||
initError error | ||
} | ||
|
||
func serveNotecard(w http.ResponseWriter, req *http.Request) { | ||
var transport = os.Getenv("NOTECARD_TRANSPORT") | ||
|
||
if req.Method != http.MethodPost { | ||
w.WriteHeader(http.StatusMethodNotAllowed) | ||
fmt.Fprintf(w, "Method not allowed") | ||
return | ||
} | ||
|
||
var data map[string]interface{} | ||
if err := json.NewDecoder(req.Body).Decode(&data); err != nil { | ||
http.Error(w, fmt.Sprintf("Error decoding JSON: %v", err), http.StatusBadRequest) | ||
return | ||
} | ||
func handleError(w http.ResponseWriter, err error, msg string) { | ||
err_str := fmt.Sprintf("%s: %v", msg, err) | ||
http.Error(w, err_str, http.StatusInternalServerError) | ||
log.Print(err_str) | ||
} | ||
|
||
for key, value := range data { | ||
fmt.Printf("%s: %v\n", key, value) | ||
func (s *server) ServeHTTP(w http.ResponseWriter, req *http.Request) { | ||
if s.card == nil || s.initError != nil { | ||
handleError(w, s.initError, "while initialising notecard") | ||
log.Fatal("Notecard not initialised, exiting...") | ||
} | ||
|
||
jsonString, err := json.Marshal(data) | ||
if err != nil { | ||
fmt.Println("Error encoding JSON:", err) | ||
w.WriteHeader(http.StatusInternalServerError) | ||
panic(err) | ||
if req.Method != http.MethodPost { | ||
w.WriteHeader(http.StatusMethodNotAllowed) | ||
log.Printf("%s: Method not allowed", w) | ||
return | ||
} | ||
|
||
// Print the resulting JSON string | ||
fmt.Println(string(jsonString)) | ||
|
||
note_rsp, err := card.Transaction(data) | ||
body, err := io.ReadAll(req.Body) | ||
if err != nil { | ||
w.WriteHeader(http.StatusInternalServerError) | ||
panic(err) | ||
handleError(w, err, "while reading request body") | ||
return | ||
} | ||
req.Body.Close() | ||
|
||
note_rsp_json, err := json.Marshal(note_rsp) | ||
note_rsp, err := s.card.TransactionJSON(body) | ||
if err != nil { | ||
fmt.Println("Error encoding JSON:", err) | ||
w.WriteHeader(http.StatusInternalServerError) | ||
panic(err) | ||
handleError(w, err, "while performing a card transaction") | ||
return | ||
} | ||
log.Printf("notecard response: %s", note_rsp) | ||
|
||
// Set the Content-Type header to application/json | ||
w.Header().Set("Content-Type", "application/json") | ||
w.Write(note_rsp_json) | ||
|
||
w.WriteHeader(http.StatusOK) | ||
_, err = w.Write(note_rsp) | ||
if err != nil { | ||
log.Printf("error writing response: %v", err) | ||
return | ||
} | ||
Bucknalla marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
func setupNotecard(protocol Transport) *notecard.Context { | ||
var card *notecard.Context | ||
var err error | ||
func setupNotecard(protocol string) (*notecard.Context, error) { | ||
log.Printf("Setting up Notecard with protocol: %s\n", protocol) | ||
|
||
fmt.Printf("Setting up Notecard with protocol: %s\n", protocol) | ||
if protocol != Serial && protocol != I2C { | ||
return nil, fmt.Errorf("unsupported transport protocol: %v", protocol) | ||
} | ||
|
||
if protocol == Transport("serial") { | ||
card, err = notecard.OpenSerial("/dev/tty.usbmodemNOTE1", 9600) | ||
if err != nil { | ||
fmt.Printf("Error opening Notecard: %v\n", err) | ||
panic(err) | ||
} | ||
} else if protocol == Transport("i2c") { | ||
card, err = notecard.OpenI2C("/dev/i2c-1", 0x17) | ||
if protocol == Serial { | ||
card, err := notecard.OpenSerial("/dev/tty.usbmodemNOTE1", 9600) | ||
if err != nil { | ||
fmt.Printf("Error opening Notecard: %v\n", err) | ||
panic(err) | ||
return nil, fmt.Errorf("error opening Notecard: %v", err) | ||
} | ||
} else { | ||
fmt.Printf("Missing transport protocol\n") | ||
return card, nil | ||
} | ||
|
||
card, err := notecard.OpenI2C("", 0x17) | ||
if err != nil { | ||
return nil, fmt.Errorf("error opening Notecard: %v", err) | ||
} | ||
|
||
status := map[string]interface{}{ | ||
"req": "card.status", | ||
} | ||
return card | ||
if _, err = card.Transaction(status); err != nil { | ||
return nil, fmt.Errorf("error querying Notecard status: %v", err) | ||
} | ||
return card, nil | ||
} | ||
|
||
func main() { | ||
var i interface{} = os.Getenv("NOTECARD_TRANSPORT") | ||
transport, err := i.(Transport) | ||
|
||
if !err { | ||
fmt.Printf("Error getting transport protocol, defaulting to I2C...\n") | ||
if transport == "" { | ||
log.Printf("transport protocol not provided, defaulting to I2C...") | ||
transport = I2C | ||
} | ||
|
||
card = setupNotecard(transport) | ||
card, err := setupNotecard(transport) | ||
if err != nil { | ||
log.Printf("while setting up notecard: %v", err) | ||
} | ||
defer card.Close() | ||
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. we don't really care given our lifecycle, but this is going to fail (panic even), if the setup above failed (because card will be nil). |
||
|
||
http.HandleFunc("/", serveNotecard) | ||
http.Handle("/", &server{card: card, initError: err}) | ||
http.ListenAndServe(":3434", nil) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
There's two stances here: either you trust that the caller have consistently and properly set the (s.card, s.initError) tuple, or you don't.
If you do, it means you only need to test either of them (any is fine, as each of them carry the meaning of success VS failure), so you only have to choose whether your prefer to test card or initError.
If you don't, they you need to test both of them, like you did. But then you need to go a bit further in the defensiveness. Since we can't trust the caller, it could very well be e.g. that they have set s.card but they have forgotten to set s.initError. In which case, your test is going to be true, but you're going to pass a nil error to handleError. So we would need to check that s.initError is not nil, etc..
Anyway, long story short, since we (2 programmers at arribada) are the caller in that case, I would choose to trust us, and I would change L32 to test only for s.initError.